11
\$\begingroup\$

Goals for the class

  • Create Immutable objects – i.e. only Getters – no Setters
  • Object creation only possible through a constructor, not via New keyword, to ensure that no objects are created without a valid state.
  • Keep the constructor method in the same code module as the class itself.

Other common solutions for VBA constructors

  • Creating a global Factory class/module that provides constructors for all creatable objects, as suggested in this post. This can be arduous to maintain, creates a dependency between the modules and arguably violates encapsulation/single responsibility principal. Does not prevent use of New keyword to create objects.

  • Creating a Factory class for each class Provides more encapsulation, less dependancy and narrower responsibility, but again does not prevent use of New keyword and soon results in a proliferation of types.

  • Providing a constructor in the class itself and make it available through a predeclared instance using the VB_PredeclaredId attribute, as discussed in this post. Better, but still allows use of New keyword, and does not prevent access to the (potentially invalid) state of the predeclared instance.

Proposed Solution

  • Use the predeclared instance of a class as the “Factory Instance". Only this predeclared instance may create other instances of the class.
  • Provide a constructor method (I use the name Make) in the class module itself. It can be called on the Factory Instance using ClassName.Make, or on another instance of the class using ObjectName.Make. Other instances delegate creation to the Factory Instance and return a new object – it doesn’t alter their own state.
  • Each time a new instance is initialised it checks if it is being made by the Factory Instance, otherwise throws a runtime error – i.e. use of New keyword is not allowed.
  • An attempt to access the state of the Factory Instance returns a runtime error.

Implementation

IMaker.cls - Interface

Option Explicit
Public Property Get IsMaking() As Boolean
   End Property

Provides an interface for objects to query the Factory Instance to check if their creation was requested. Put in a separate interface so that it does not show up in the public members of the class. Same interface can be used by all classes that use this pattern.

Point.cls - Edited in Notepad so that Attribute VB_PredeclaredId = True

Example class with two properties: X and Y. Verbose comments added for illustrative purposes

Option Explicit
Private Const CLASS_NAME As String = "Point"

Private Maker As IMaker
Private IsMaking As Boolean
Private X_ As Double
Private Y_ As Double

Implements IMaker

Private Sub Class_Initialize()
    ' Check Instance isn't the Factory Instance (which can be created without checks)
    If Not Me Is Point Then
        ' Create a reference to the Factory Instance, cast as type IMaker
        Set Maker = Point
        ' Throw Runtime error if creation wasn't requested by Factory Instance
        If Not Maker.IsMaking Then ThrowError_AttemptToCreateInstanceOutsideOfConstructor
    End If
    End Sub

Public Function Make(ByVal X As Double, ByVal Y As Double) As Point
    If Me Is Point Then         ' Object is Factory Instance
        ' Allow new instances to be created
        IsMaking = True
        ' Create a new instance
        With New Point
            ' Pass parameters to its constructor and return new object
            Set Make = .Make(X, Y)
        End With
        ' Disallow new instances to be created
        IsMaking = False
    ElseIf Maker.IsMaking Then  ' Object is new instance being created by the Factory Instance
        ' Set state and return self
        X_ = X
        Y_ = Y
        Set Make = Me
    Else
        ' Delegate creation of new object to Factory Instance and return new object
        Set Make = Point.Make(X, Y)
    End If
    End Function

Public Property Get X() As Double
    ' Disallow access to state of Factory Instance
    If Me Is Point Then ThrowError_AttemptToAccessPredeclaredInstance
    ' Return state
    X = X_
    End Property

Public Property Get Y() As Double
    ' Disallow access to state of Factory Instance
    If Me Is Point Then ThrowError_AttemptToAccessPredeclaredInstance
    ' Return state
    Y = Y_
    End Property

Private Property Get IMaker_IsMaking() As Boolean
    ' Indicate whether new instances of class can be created or not - only ever set to True by Factory Instance
    IMaker_IsMaking = IsMaking
    End Property

Private Sub ThrowError_AttemptToCreateInstanceOutsideOfConstructor()
    Err.Raise VBA.vbObjectError + 513, CLASS_NAME, "Cannot create instance of " & CLASS_NAME & " via New"
    End Sub

Private Sub ThrowError_AttemptToAccessPredeclaredInstance()
    Err.Raise VBA.vbObjectError + 514, CLASS_NAME, "Cannot access state of predeclared instance of " & CLASS_NAME
    End Sub

Example Usage

Sub ExampleUsage()

    Dim Point1 As Point, Point2 As Point

    Set Point1 = Point.Make(1, 2)   ' Creates new object via Factory Instance
    Set Point2 = Point1.Make(3, 4)  ' Creates new object via existing object
    Set Point1 = Point1.Make(5, 6)  ' Creates new object and assigns to Point1 - does not change state of previous object

    Set Point1 = New Point          ' Throws Runtime Error

    Dim Point3 As New Point         ' Does not throw Error - Instantiation only occurs on first use
    Debug.Print ObjPtr(Point3)      ' Throws Runtime Error

    Debug.Print Point.X             ' Throws Runtime Error - cannot access state of Predeclared instance

    Point1.X = 5                    ' Throws Compiler Error - only GET methods provided

    End Sub

Drawbacks

This is obviously quite a bit of “boilerplate” for a simple object, but VBA’s lack of features seems to require it. I would welcome suggestions to streamline it. Putting a check in each method that the object isn’t the Factory Instance obviously adds overhead and verbosity – can be omitted if you don’t care too much about code accessing the state of the predeclared instance.

\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks for this post, I really enjoyed it - it reminded me of this code of mine, which turned into this Rubberduck New article about OOP/SOLID in VBA. \$\endgroup\$ – Mathieu Guindon Mar 11 '17 at 3:07
  • 1
    \$\begingroup\$ I have rolled back your recent edits. Please do not update your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version of your code or comments on feedback in your question. Please see what you may and may not do after receiving answers. If you feel this is inappropriate please visit Code Review Meta to discuss it. \$\endgroup\$ – Der Kommissar Mar 11 '17 at 16:25
  • \$\begingroup\$ Sorry, am used to other sites where adding an addendum edit is correct etiquette. Thanks for link to that advice. I had googled looking for that information before I edited but could find an answer. \$\endgroup\$ – Mark.R Mar 11 '17 at 16:28
  • \$\begingroup\$ @Mark.R That's understandable, no harm against you. :) We try to keep things as fair as possible for all users, askers and answerers. Hopefully we see more good questions from you in the future, and maybe even some answers! :) \$\endgroup\$ – Der Kommissar Mar 11 '17 at 16:31
7
\$\begingroup\$

Indentation

The non-standard end-of-block token indentation is off-putting; this is the first time I see VB code that doesn't align the start and end token columns of code blocks:

Public Sub DoSomething()
    'instructions
    End Sub

As opposed to:

Public Sub DoSomething()
    'instructions
End Sub

Seems you go out of your way to fight the IDE to consistently apply that style - I appreciate the consistency, but I find it awkward when If...End If blocks don't seem to follow the same rule - I would expect this:

If condition Then
    'instructions
    End If

But you have them the standard way, like this:

If condition Then
    'instructions
End If

Your indenting style is your own preference, sure, but there's also commonly established conventions. Smart Indenter is an automatic indentation add-in that has been around for almost two decades (ported to .net and supporting x64 hosts through Rubberduck, an open-source project I'm working on - kudos to @Comintern for the work on the indenter); it offers a TON of indenting options, and none of them support that style - I'd take it as a sign.


Readability

The state handling and instance checks work, but it takes a very attentioned reader to follow; the back-and-forth between the default instance and the new object being created, the assignment of Me pointers, the IsMaking flag... the result is a rather convoluted execution path.

It works, but it's hard to follow.


IMaker

The interface is superfluous; while it's nice that you're formalizing the "I'm making something" state, the only calls to IsMaking are, as a comment explains, made from the default instance of the very same class that implements the interface - because no code is ever working off an IMaker instance (and/or accessing that instance from its IMaker interface), the interface is ultimately useless.


Error Handling

I like what I'm seeing: private procedures dedicated to raising specific runtime errors. The CLASS_NAME constant could be easily eliminated with a call to TypeName(Me) though.

However the calling code has no easy way of telling VBA.vbObjectError + 513 from VBA.vbObjectError + 514 without hard-coding these magic values; the class could expose a public enum for this:

Public Enum PointFactoryError
    ERR_ClassIsNotCreatable = VBA.vbObjectError + 513
    ERR_InstanceStateNotAvailable 'next value has to be VBA.vbObjectError + 514
End Enum

That way you can raise them without resorting to magic numbers:

Private Sub ThrowError_AttemptToCreateInstanceOutsideOfConstructor()
    Err.Raise ERR_ClassIsNotCreatable, TypeName(Me), "Cannot create instance of " & TypeName(Me) & " via New"
End Sub

Note that this message is very similar to the compile-time error Invalid use of 'New' keyword; I'd probably word the error message in a similar fashion.

And now callers can handle that error, also without resorting to magic numbers:

ErrHandler:
    If Err.Number = ERR_ClassIsNotCreatable Then
        '...
    End If

..which feels pretty weird, given that run-time error should really be a compile-time error, which means that error would only ever be raised in the case of a bug introduced by the programmer.

Perhaps Debug.Assert calls would be a better alternative then:

Debug.Assert Not Me Is Point 'breaks on-the-spot if assertion fails

Immutability

In VB6 you would set the VB_Creatable attribute to False and you had a class that you couldn't New up, but while this attribute appears in an exported class module, it has no effect whatsoever in VBA.

I like how your class is aware of its default instance, and uses this knowledge to forbid access to that instance's state. It's a bit combersome though, but if you're in a VBA project without other VBA project references, it's pretty much the only way to go.

VBA classes can only ever be New'd up within the project they're in. This means you get the Attribute VB_Creatable = False behavior for free, if you put such immutable types in their own "framework" VBA project.

If having a "framework/toolbox add-in" project is an option (an immutable Point type seems a pretty useful type to have in one's toolbox), then there's another, much simpler solution: access modifiers.

The Friend modifier means a member can only be accessed from within the project it's in. This means you can very well have this:

Public Property Get X() As Double
    X = X_
End Property

Friend Property Let X(ByVal value As Double)
    X_ = value
End Property

And then any VBA project that uses this type will never know about a Property Let member. Combined with the fact that the class can't be New'd up because it's declared in another referenced project, the result is effectively an immutable instance.

You could also force the client code to work with an interface for the type - say IPoint:

Option Explicit

Public Property Get X() As Double
End Property

Public Property Get Y() As Double
End Property

And then you use the default instance of Point as a factory for IPoint instances - here's how I would have done it:

Private Type TPoint
    X As Double
    Y As Double
End Type
Private this As TPoint
Implements IPoint

Public Function Create(ByVal X As Double, ByVal Y As Double) As IPoint
    With New Point
        .X = X
        .Y = Y
        Set Create = .Self
    End With
End Function

Friend Property Get Self() As Point
    Set Self = Me
End Property

Friend Property Let X(ByVal value As Double)
    this.X = value
End Property

Friend Property Let Y(ByVal value As Double) 
    this.Y = value
End Property

Private Property Get IPoint_X() As Double
    IPoint_X = this.X
End Property

Private Property Get IPoint_Y() As Double
    IPoint_Y = this.Y
End Property

The private type TPoint allows me to have backing fields with the exact same name as the property that exposes them, without any funky prefix or suffix - and all backing field calls are still immediately obvious, because they're all member access calls of the this private field, which is responsible for holding the encapsulated instance state.

And if you ever need to serialize a Point instance, you could always have these, assuming the type implements some ISerializable interface:

Private Property Get ISerializable_Size() As Long
    ISerializable_Size = LenB(this)
End Property

Private Sub ISerializable_Serialize(ByVal fileNumber As Long)
    Put# fileNumber, this
End Sub

Private Function ISerializable_Deserialize(ByVal fileNumber As Long) As Object
    Dim value As TPoint
    Get# fileNumber, value
    Set ISerializable_Deserialize = Create(value.X, value.Y)
End Sub

But I'm drifting...

From the calling code (in another VBA project), the members of the Point class are:

  • Public Function Create(ByVal X As Double, ByVal Y As Double) As IPoint

...and that's absolutely all.

To access the instance state, the calling code needs to work against the IPoint interface:

Dim p As IPoint
Set p = Point.Create(42, 17)

...which only exposes Property Get members for X and Y properties: the API is as clean as it gets, and as a bonus you get this for free:

Dim p As Point
Set p = New Point 'compile error

Of course if the type must be defined in the same VBA project that consumes it, then things get messier, because VBA will let you New it up and you have access to the Friend members of the default instance, so you might want to add instance checks to explicitly prohibit this... like you did; but even if it's consumed in the same project, the calling code should always be working against the IPoint interface, so the default instance state would never be used anyway.

\$\endgroup\$
  • \$\begingroup\$ Thanks Mat, really appreciate the long and thoughtful response. I've added general comments to my post above, but I wanted to pick up on one particular thing you said - that the IMaker interface isn't accessed by any code and is ultimately useless. I'm not sure I agree - each non-default instance calls the IsMaking method of theIMaker interface of the default instance. It's the channel of communication between the default and non-default instances. Without it the IsMaking method would need to be a public member of the class. Perhaps I'm miss-understanding what you meant? \$\endgroup\$ – Mark.R Mar 11 '17 at 14:56
  • \$\begingroup\$ I didn't say IsMaking was useless, I said the fact that it's an interface member makes no difference at all. You're calling it from the default (concrete) instance, not from the interface. Also please avoid editing your question in response to answers. \$\endgroup\$ – Mathieu Guindon Mar 11 '17 at 16:04
  • \$\begingroup\$ @Mark.R I noticed you mentioned folders in your (rolled back) edit - indeed VBA's project explorer doesn't support it.. However you might be interested in Rubberduck's annotation system, which gives you any folder hierarchy you can dream of, in Rubberduck's code explorer (I manage this project). \$\endgroup\$ – Mathieu Guindon Mar 12 '17 at 17:06
4
\$\begingroup\$

EDIT Mat's Mug has the best approach in going for an Interface and an add-in, but if you constrain the requirements to a self-contained solution, and you assume the user of the class is unfamiliar with interfaces, then you can have everything in a single class.

Your predeclared instance has the private fields and property gets to facilitate passing the variables to new instances. I've added a State property (replacing your IsMaking property) with a Friend scope (should the class end up in an add-in), and I've used VBA's ability to hide an emum member by prefixing it with an underscore, to obfuscate the values that State could otherwise be (i.e. Consumers of the class don't need to know what State does, and only know that State can be Ready.

Setting up some Enums

As @Mat'sMug pointed out, the error values could use their own Enum:

Public Enum pointError
  errNotCreatable = vbObjectError + 100
  errOnlyCallableFromDefault = vbObjectError + 101
End Enum

And, I'll use an Enum for setting the state of the predeclareed instance. It's either ready, or it's Initializing a new Point. Note the use of a leading _ to make the non-default enum member hidden, and the use of square-brackets to make the syntax valid.

Public Enum pointState
  Ready = 0
  [_InitializingNewPoint] = 1
End Enum

Making fields more friendly

Then, again as per Mat'sMug's suggestion, I'm using a private type to store fields:

Private Type TPoint
  X As Double
  Y As Double
  State As pointState
End Type

Private this As TPoint

Hijacking the predeclared instance's fields

In the factory method Create, I temporarily set the state and X/Y fields of the default instance, and then in the Class_Initialize event of the new instance, I read those fields though the default instance's getters. As such, I've relaxed the conditions around being able to access the default instance's getters, but this could easily be re-added. The default Point will always have X and Y's getters return 0, in much the same way that the default Integer will always have default value 0.

    Public Function Create(X As Double, Y As Double) As Point
      If Me Is Point Then
        this.State = pointState.[_InitializingNewPoint]
        'Hijack the default instance's X and Y properties
        this.X = X
        this.Y = Y
        Set Create = New Point
        'Restore the default instance's X and Y properties
        this.X = 0
        this.Y = 0
        this.State = pointState.Ready
      Else
        Err.Raise pointError.errOnlyCallableFromDefault, "Point", "Invalid instance use of Create - Use Point.Create to instatiate new Point objects."
      End If
    End Function

Complete Code

This is a pre-declared class. I require that new instances of Point be created from the default instance, and only the default instance of Point (which means that the default instance could do useful things like tracking the number of Point instances), but it isn't much work to facilitate any instance of Point being a valid factory.

Option Explicit

Public Enum pointError
  errNotCreatable = vbObjectError + 100
  errOnlyCallableFromDefault = vbObjectError + 101
End Enum

Public Enum pointState
  Ready = 0
  [_InitializingNewPoint] = 1
End Enum

Private Type TPoint
  X As Double
  Y As Double
  State As pointState
End Type

Private this As TPoint

Private Sub Class_Initialize()
  If Not Me Is Point Then
    If Point.State = pointState.[_InitializingNewPoint] Then
      this.X = Point.X
      this.Y = Point.Y
    Else
      Err.Raise pointError.errNotCreatable, "Point", "Invalid use of New Keyword - Use Point.Create to instatiate new Point objects."
    End If
  End If
End Sub

Public Property Get X()
  X = this.X
End Property

Public Property Get Y()
  Y = this.Y
End Property

Friend Property Get State() As pointState
  State = this.State
End Property

Public Function Create(X As Double, Y As Double) As Point
  If Me Is Point Then
    this.State = pointState.[_InitializingNewPoint]
    'Hijack the default instance's X and Y properties
    this.X = X
    this.Y = Y
    Set Create = New Point
    'Restore the default instance's X and Y properties
    this.X = 0
    this.Y = 0
    this.State = pointState.Ready
  Else
    Err.Raise pointError.errOnlyCallableFromDefault, "Point", "Invalid instance use of Create - Use Point.Create to instatiate new Point objects."
  End If
End Function

Usage:

  Dim p As Point
  Set p = Point.Create(1, 2)
  Debug.Print p.X, p.Y

  Dim o As New Point
  Debug.Print ObjPtr(o)  'Throws: Invalid use of New Keyword - Use Point.Create to instatiate new Point objects.

  Dim q As Point
  Set q = New Point      'Throws: Invalid use of New Keyword - Use Point.Create to instatiate new Point objects.

  Dim r As Point
  Set r = p.Create(1, 2) 'Throws: Invalid instance use of Create - Use Point.Create to instatiate new Point objects.
\$\endgroup\$
  • \$\begingroup\$ IMO this still has OP's biggest problem: a stateful "static context", and context-switching between default instance and new object. \$\endgroup\$ – Mathieu Guindon Mar 11 '17 at 2:05
4
\$\begingroup\$

Naming

I'll start off with a small style gripe (the indentation is the big one, but that's already been covered). Your naming convention for your private instance members is confusing. This...

X = X_

... is visually indistinguishable from a line continuation at first glance:

X = X _

That severely increases the cognitive load of reading the code. I don't want to have to put conscious effort into trying to decipher the basic structure of the code when I'm reading it. IMHO, this naming convention is worse than X = foo and Y = bar because I instinctually glance down to the next line and think "WTH is the End Property doing there? Oh yeah - not a line continuation". Hungarian notation is better. About the only thing that comes to mind that would be worse is non-breaking spaces or other non-printable characters. OK, flame mode off. :Whew:


Everything Other Than Naming

First, I have to say that I share @Mat's Mug's concern about trading compile errors for run-time errors. If immutably is a responsibility of the class (totally encapsulating members it doesn't want you to have access to), then it shouldn't place additional burdens on the calling code in terms of checking for run-time errors. In general, I think it's a bad idea for code that is syntactically correct to throw because you want to restrict the use of a syntax.

Second, attaching the concept of "state" to the predeclared instance not only runs counter to expected behavior of a predeclared instance, it could lead to issues if anything is making assumptions about the state, because...

Unload Point

...and...

Set Point = Nothing

So, this basically gets back to the question of "how do I create an instance of a class and assign values to read-only properties"? A good clue comes from .NET, where objects have access to each other's private members (that sounds dirtier than I intended) from within the class. Just find a way to set them.

Which leads us to... vtable hacking. The grossly simplified explanation is that an interface is simply a collection of function pointers to the entry points of the class's functionality. The instance of the class is passed as a parameter (along with any others) to that function. So you just swap the vtable entries of your private initializer (which sets the instance variables) with your factory method, set them, and swap them back:

WARNING: This is 32 bit specific code - do not run it in a 64 bit host or it will crash, and is only meant to illustrate the concept. On a 64 bit machine, the offset calculations in the vtable are different. Do not run this without saving your work. It could take down the host VBA application. It could take down Windows. I take no responsibility for what you do with this code - run this at your own risk.

'Point.cls - Predeclared
Option Explicit

Private Declare Sub CopyMemory Lib "kernel32.dll" Alias "RtlMoveMemory" _
    (hpvDest As Any, hpvSource As Any, ByVal cbCopy As Long)

Private Type TPoint
    X As Double
    Y As Double
End Type

Private this As TPoint

Public Function Make(ByVal X As Double, ByVal Y As Double) As Point
    Dim newPoint As Point
    Set newPoint = New Point
    'This swaps the vtable pointer of this function to the private internal one.
    SwapFunctions
    'Make the call. Due to the vtable hack, this will call InternalInitialize.
    newPoint.Make X, Y
    'Reset the vtable to it's initial state.
    SwapFunctions
    Set Make = newPoint
End Function

Public Property Get X() As Double
    X = this.X
End Property

Public Property Get Y() As Double
    Y = this.Y
End Property

'This has to be the exact same signature as Make or it will mis-align the stack
Private Function InternalInitialize(ByVal X As Double, ByVal Y As Double) As Point
    this.X = X
    this.Y = Y
End Function

Private Sub SwapFunctions()
    Dim first As Long
    Dim second As Long

    'The ObjPtr doesn't matter here - it's the interface, not the instance that matters.
    CopyMemory first, ByVal ObjPtr(Me), 4
    CopyMemory second, ByVal first + &H1C + 8, 4
    CopyMemory ByVal first + &H1C, ByVal first + &H1C + 12, 4
    CopyMemory ByVal first + &H1C + 12, second, 4
End Sub

Calling it is simple - use the factory method to get a new instance. If you use New, you'll just get an immutable object back with and X of 0 and a Y of 0:

Public Sub Example()
    Dim p As Point
    Set p = Point.Make(42, 42)
    Debug.Print p.X, p.Y

    Set p = New Point
    Debug.Print p.X, p.Y
End Sub

Additional caveats - the vtable swap is entirely dependant on the order of the functions in the code above. Switching the order will break it. Adding additional public members will break it. If you do something like this in a production environment, consider this code a proof of concept and then make it a hell of a lot more robust.

NOTE: @Thungerframe provided a link that goes through some of the concepts involved here, which might be worth reading:

http://www.vbforums.com/showthread.php?781595-VB6-Call-Functions-By-Pointer-(Universall-DLL-Calls)

If you're interested in kind of methodology, I'd also suggest reading Advanced Visual Basic 6 by Matthew Curland.


EDIT:

The following changes should make this safe on either 32 or 64 bit hosts (untested due to lack of access to a 64 bit Office install). Declare the following constants:

#If Win64 Then
    Private Const POINTER_SIZE As LongPtr = 8
#Else
    Private Const POINTER_SIZE As LongPtr = 4
#End If
Private Const INTERFACE_OFFSET As LongPtr = 7 * POINTER_SIZE
Private Const MAKE_FUNCTION_OFFSET As LongPtr = INTERFACE_OFFSET + (2 * POINTER_SIZE)
Private Const INIT_FUNCTION_OFFSET As LongPtr = INTERFACE_OFFSET + (3 * POINTER_SIZE)

Change the Declare Function to this:

Private Declare PtrSafe Sub CopyMemory Lib "kernel32.dll" Alias "RtlMoveMemory" _
    (hpvDest As Any, hpvSource As Any, ByVal cbCopy As Any)

And replace Sub SwapFunctions with this implementation:

Private Sub SwapFunctions()
    Dim first As Long
    Dim second As Long

    'The ObjPtr doesn't matter here - it's the interface, not the instance that matters.
    CopyMemory first, ByVal ObjPtr(Me), POINTER_SIZE
    CopyMemory second, ByVal first + MAKE_FUNCTION_OFFSET, POINTER_SIZE
    CopyMemory ByVal first + INTERFACE_OFFSET, ByVal first + INIT_FUNCTION_OFFSET, POINTER_SIZE
    CopyMemory ByVal first + INIT_FUNCTION_OFFSET, second, POINTER_SIZE
End Sub
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.