A post on Code Review recently caught my attention (emphasis mine):
If you are setting up a class, don’t encapsulate a
Typeinside of it – you are only repeating what a class does! I am not sure where this anti-pattern comes from.
The author of these words didn’t use the term “anti-pattern” in the same way I would have… They didn’t mean it as the toxic coding practices I use it for (I know, I asked!). But they aren’t seeing the benefits of it, and ultimately consider it clutter… and that’s where we disagree, regardless of whether “anti-pattern” is incendiary wording or not.
If you’ve been reading this blog for some time, you’ve probably noticed this rather consistent (VBA code written before 2015 doesn’t count!) pattern in my writing of class modules: whenever I need a class, I start by declaring a
Private Type for its private instance fields, always named after the class module itself and prefixed with an admittedly rather “Hungarian”
T prefix; then the only actual private field in the class is a
Private this variable, like this:
Option Explicit Private Type TPerson FirstName As String LastName As String End Type Private this As TPerson Public Property Get FirstName() As String FirstName = this.FirstName End Property Public Property Let FirstName(ByVal value As String) this.FirstName = value End Property Public Property Get LastName() As String LastName = this.LastName End Property Public Property Let LastName(ByVal value As String) this.LastName = value End Property
The same class module would “normally” look something like this:
Option Explicit Private mFirstName As String Private mLastName As String Public Property Get FirstName() As String FirstName = mFirstName End Property Public Property Let FirstName(ByVal pFirstName As String) mFirstName = pFirstName End Property Public Property Get LastName() As String LastName = mLastName End Property Public Property Let LastName(ByVal pLastName As String) mLastName = pLastName End Property
Yes, it’s less code. So what’s my problem with it?
- Properties and their respective backing field don’t (can’t) use the same identifier.
mprefix is pure clutter that’s only there to say “hey look, this is a private field /module variable!” – in other words, it’s Systems Hungarian notation and does nothing other than increase the cognitive load. Even worse with an underscore, which wrecks the consistent camelCase/PascalCase conventions of literally everything written in any VB dialect.
- It’s not true that using such Hungarian prefixes helps with autocompletion and IntelliSense. If the class has 5 properties that happen to start with a
M, then your 5 backing fields are intertwined with 10 public members (so, drowned, really) that also start with an
- Mutator parameters aren’t consistent either. That
pprefix is just as annoying, and I’ll go as far as to say that this
p-for-parameter convention is exactly what’s behind the fact that many VBA programmers have never dared implementing a class module “because it’s too confusing” and hard to follow.
- The locals debugging toolwindow becomes cluttered with all the private fields duplicating the
Property Getmembers‘ values.
With my “anti-pattern”, there’s a little bit more code, yes. But:
- Properties and their respective backing field consistently use the same identifier. IntelliSense / autocomplete for my fields consistently only ever includes the backing fields, and all I had to do was to type
- No need for any Hungarian prefix anywhere. I use
Tfor the type declaration (I also use
Ifor interfaces, like in .NET and most C-based languages), because I find that using the class identifier (which would be perfectly legal) would be potentially confusing in
Private this As Class1, since in any other context (outside the class module itself) the identifier
Asclause would be referring to the
- Parameter names are always explicitly passed
value. Yes, this makes
Range.Valueshow up as
Range.value, but VBA being case-insensitive, it makes no difference whatsoever. I could have used any other identifier, but
valueis what VB.NET and C# use; besides
RHSisn’t quite as sexy, if more semantically correct. But naming parameters after the property member is an objectively horrible idea; all you see is a soup of
Foowith assignment operators in between.
- The locals debugging toolwindow now nicely regroups all the fields under
this, so the object’s state is much easier to browse and understand at a glance.
- If you ever need to serialize an object’s state to a binary file, then all you need to do is to
Put #fileHandle thisand you’re done. The inverse process is just as simple: no need to enumerate the properties one by one, convert them, or manipulate them in any way.
I’d love to hear exactly what’s wrong with this “anti-pattern” of mine – I’ve grown pretty fond of it in the past couple years, and until someone can show me how and why I’m actively hurting something somewhere with it, I’ll keep using it in my own code, and posting Code Review and Stack Overflow answers featuring it.. and my blog posts will keep using it too.
One concern raised, was that a UDT doesn’t play well with collections. But this UDT isn’t going to end up in a collection anytime soon – and even if the class instance went into a collection, the encapsulated UDT couldn’t care less: all it does is regrouping the class’ internal state. Code outside the class doesn’t know about it, and couldn’t if it wanted.
You might be worried that a UDT incurs additional overhead… but it doesn’t: it simply provides a convenient structure to organize the private fields of a class. Two
Long private fields allocate 4 bytes each and total 8 bytes; a UDT with two
Long members allocates a total of 8 bytes, as
Len(this) shows. What’s an easy way to know how much space the instance fields of a class take up?
Rubberduck has an encapsulate field refactoring that makes a public field private, renames it, and introduces
Property Get and appropriate
Set mutators for it.
For a while I’ve been considering implementing a feature that builds on this
Private Type [anti?] pattern, but held back because I didn’t want Rubberduck to enforce my coding style… although… I would love to be able to just declare my private type and my
this private field, parse, and then right-click the UDT field and have Rubberduck generate all the
Set boilerplate for me.
Would that make it more compelling?