Rubberduck Style Guide

As Rubberduck started to beef up its static code analysis capabilities in late 2015, it became evident that writing VBA (or VB6) code with Rubberduck loaded up in the Visual Basic Editor (VBE) would inevitably change not only how we work in VBA, but also how we write our VBA code in the first place.

Rubberduck is essentially providing a bridge between VBA land where people just get in and have a go and the VS land where if you don’t know a great deal about software development, you just waste your time and burn. Rubberduck will put a lot of people on a big learning curve and this will result in a lot of questions.” – AndrewM- commented on Oct 9, 2015

There’s an old issue (#823, still opened as of this writing) about having a coding style guide somewhere, that would enshrine the philosophy behind what Rubberduck is, in a way, trying to make your code-writing be/become; I think that was a great idea and I’m hoping this post captures the essence of it, at least as far as thinking code goes.


About Code Inspections

If you fire up Rubberduck on any legacy VBA project with any significant amount of code, there’s a very high probability that static code analysis generates tons of inspection results, for various mundane little things. Should your goal be to quick-fix all the things and have code that doesn’t spawn any Rubberduck inspection results?

Perhaps surprisingly, the answer is a resounding “no”.

Severity Levels

In Rubberduck each inspection has a configurable “severity level” that defaults to Warning for most inspections (it’s the default-unless-specified-otherwise for all Rubberduck inspections):

  • Error level indicates a potential problem you likely want to pay immediate attention to, because it could be (or cause) a bug. If inspection results rendered in the code pane, these would be red squiggly underlines.
  • Warning level indicates a potential issue you should be aware of.
  • Suggestion level is usually used for various opportunities, not necessarily problems.
  • Hint level is also for various non-problematic opportunities. If inspection results rendered in the code pane, these would be a subtle dotted underline with a hover text.
  • DoNotShow disables the inspection: not only its results won’t show, they won’t even be generated.

By default, Rubberduck is configured to run all (that’s currently over 110, counting the hidden/Easter egg ones) inspections, with a handful of cherry-picked exceptions for inspections that would be flagging the exact opposite situation that another enabled inspection is already flagging – for example we ship implicit ByRef modifier enabled (as a Hint), but redundant ByRef modifier is disabled unless you give it a severity level that’s anything other than DoNotShow. This avoids “fixing” one inspection result only to get a new one flagging the exact opposite, which would be understandably confusing for users that aren’t familiar with static code analysis tooling.

Are inspections somehow imbued with the knowledge of whether you should treat them as errors, warnings, or mere hints and suggestions? Sometimes, yes. Missing Option Explicit should make a clear consensus at Error level. On the flipside, whether an implicit default member call or the use of an empty string literal should be a Warning, a Hint, or shown at all probably depends more on how comfortable or experienced you are with VBA/VB6 as a language, or could be just a personal preference; what matters is that the static code analysis tooling is letting you know something about the code, that the code alone isn’t necessarily saying.

Philosophy

One of the very first inspection to be implemented in Rubberduck was the Option Explicit inspection. Okay, part of it was just because it was a trivial one to implement even before we had an actual parser… but the basic idea was (and still is) that nobody knows everything, and it’s with our combined knowledge that we make a mighty bunch, and that is why static code analysis in Rubberduck explains the reasoning behind each inspection result: there are quite many things Rubberduck warns of, that I had no idea about 10 or 15 years ago. That never stopped me (and won’t stop you either) from writing VBA code that worked perfectly fine (except when it didn’t), but whether we realize and accept it or not… a macro written in VBA code is a set of executable instructions, which makes it a program, which makes the act of writing it programming, which makes us programmers.

Being programmers that write and maintain VBA code does set us apart, mostly because the language isn’t going anywhere and the IDE is becoming more and more severely outdated and under-featured as years pass. Yet if the volume of VBA questions on Stack Overflow means anything, VBA is still very much alive, still very much being learned, and this is where Rubberduck and static code analysis comes in.

When I started learning about .NET and C# over a decade ago, there was this exciting new language feature they called LINQ for Language-INtegrated-Query where you could start querying object collections pretty much literally like you would a database, and it was awesome (still is!). In order to make this possible, the C# compiler and the .NET framework and runtime itself had to undergo some very interesting changes Jon Skeet covers in details, but the point is… the new syntax was a bit off-putting at first, and came with new and important implications (closures, deferred execution), and the company I worked for gave us all a ReSharper license, and that is how and when I discovered that thorough & accurate static code analysis tooling could be a formidable educational tool.

And I want Rubberduck to be like that, to be the companion tool that looks at your code and reveals bits of trivia, hints like “hey did you know this conditional assignment could be simplified?“, or “if that condition was inverted you wouldn’t need this empty block here“.

Maybe we don’t agree about Hungarian Notation, and that’s fine: Rubberduck wants you to be able to find it and rename it if that’s what you want to do, but you can mute that particular inspection anytime. But I believe the tool should tell you what Systems Hungarian notation is when it calls it out, and perhaps it should even explain what Apps Hungarian is and give examples, because Apps Hungarian notation absolutely is useful and meaningful (think o-for-OneBased, or src-for-Source and dst-for-Destination prefixes). But str-for-String, lng-for-Long, o-for-Object is different, in a bad kind of way.

Rubberduck flags obsolete code constructs and keywords, too. Global declarations, On Local Error statements, explicit Call statements, While...Wend loops, all have no reason to exist in brand new, freshly-written VBA code, and quick-fixes can easily turn them into Public declarations, On Error statements, implicit Call statements (without the Call keyword!), and Do While...Loop structures.

Rubberduck wants to push your programming towards objectively, quantitatively better code.

About Code Metrics

Rubberduck could count the number of lines in a procedure, and issue an inspection result when it’s above a certain configurable threshold. In fact, things are slowly falling into place for it to eventually happen. But we wouldn’t want you to just arbitrarily cut a procedure scope at 20 lines because an inspection said so! Rubberduck can measure line count, nesting levels, and cyclomatic complexity. These metrics can be used to identify problematic areas in a code base and methodically split up large complex problems into measurably much smaller and simpler ones.

Line Count simply counts the number of lines. Eventually this would expand into Statements and Comments counts, perhaps with percentages; 10% comments is probably considered a good sign, for example. But no tool is going to tell you that ' increments i is a useless comment, and even the best tools would probably not tell the difference between a huge ' the following chunk of code does XYZ banner comment and an actually valuable comment. Common wisdom is to keep this line count metric down as much as possible, but one should not do this at the expense of readability.

Nesting Levels counts the number of… well, nesting levels. While nesting two For...Next loops to iterate a 2D array (or a Range of cells) down and across is probably reasonable, further nesting is probably better off made implicit through a procedure call. Rule of thumb, it’s always good idea to pull the body of a loop into its own parameterized procedure scope. Arrow-shaped code gets flattened, line count gets lower, and procedures become more specialized and have fewer reasons to fail that way.

Cyclomatic Complexity essentially calculates the number of independent execution paths in a given procedure (wikipedia). A procedure with a cyclomatic complexity above 5 is harder to follow than one with a complexity of 1 or 2, but it’s not uncommon for a “God procedure” with nested loops and conditionals to measure in the high 40s or above.

The code metrics feature will eventually get all the attention it deserves, but as with inspections the general idea is to highlight procedures that could be harder to maintain than necessary, and nudge our users towards:

  • Writing more, smaller, more specialized procedure scopes.
  • Passing parameters between procedures instead of using global variables.
  • Having more, smaller, more cohesive modules.

Navigating the VBE

You may or may not have noticed, but the Visual Basic Editor is nudging you in the exact opposite direction, because…

  • Having fewer, larger, more general-purpose procedures puts you in a scripting mindset.
  • Using globals instead of passing parameters around is perhaps a simpler thing to do.
  • Having fewer, larger, more general-purpose modules makes it simpler to share the code between projects, and arguably easier to find things in the Project Explorer.

If you’re actually writing a small script, you can and probably should absolutely do that.

But if you’re like me then you’ve been pushing VBA to do things it wasn’t really meant to do, and you’re maintaining actual applications that could just as well be written in any other language out there, but you’re doing it in VBA because [your reasons are valid, whatever they are].

And that’s kind of a problem, because the VBE seems to actively not want you to write proper object-oriented code: its navigation tooling indeed makes it very hard to work in a project with many small modules, let alone an OOP project involving explicit interfaces and high abstraction levels.

Rubberduck lifts pretty much all the IDE limitations that hinder treating a VBA project as more than just an automation script. Now you can have a project with 135 class modules, all neatly organized by functionality into folders that can contain any module type, so a UserForm can appear right next to the classes that use it, without needing to resort to any kind of ugly prefixing schemes. You can right-click on an abstract interface (or one of its members) and quickly find all classes that implement it. You get a Find symbol command that lets you quickly navigate to literally anything that has a name, anywhere in the project. Curious about the definition of a procedure, but don’t want to break your flow by navigating to it? Peek definition (currently only in pre-release builds) takes you there without leaving where you’re at.

The Peek Definition command pops a floating panel conveniently showing the source code for the user-defined module or member you’ve selected.
Find all References shows all the places a given identifier is being used, and shows it in context so you can easily locate the specific usage you’re looking for – and then a double-click takes you there.
The Find all Implementations command is incredibly useful in object-oriented projects that leverage polymorphism through abstract interfaces: quickly locate and navigate to any implementation of any interface (class or member).

The VBE’s Project Explorer aims to give you a bird’s eye view of your project, regrouping modules by module type which is great for a small script that can get away with a small number of components, but that makes it very hard to manage larger projects. With Rubberduck’s Code Explorer you get to drill down to member level, and regroup modules by functionality using an entirely customizable folder hierarchy:

The Code Explorer leaves the VBE’s Project Explorer in the dust, fair & square.

These navigational enhancements greatly simplify moving around a project of any size, although some of them might feel a bit overkill in a smaller project, and some of them are only useful in more advanced OOP scenarios. Still, having more than just a text-based search to look for things is very useful.


Guidelines

If there’s one single over-arching principle guiding everything else, it would have to be write code that does what it says, and says what it does. Everything else seems to stem from this. These are warmly recommended guidelines, not dogma.

Naming

  • Use PascalCase if you like. Use camelCase if you like. Consistency is what you want to shoot for, and in a case-insensitive language that only stores a single version of any identifier name it’s much easier and simpler to just use PascalCase everywhere and move on to more interesting things, like tabs vs spaces.
  • Avoid _ underscores in identifier names, especially in procedure/member names.
    • Causes compile errors with Implements.
  • Use meaningful names that can be pronounced.
  • Avoid disemvoweling (arbitrarily stripping vowels) and Systems Hungarian prefixing schemes.
  • A series of variables with a numeric suffix is a missed opportunity to use an array.
  • A good identifier name is descriptive enough that it doesn’t need an explainer comment.
  • Use a descriptive name that begins with a verb for Sub and Function procedures.
  • Use a descriptive name (a noun) for Property procedures and modules.
  • For object properties, consider naming them after the object type they’re returning, like Excel.Worksheet.Range returns a Range object, or like ADODB.Recordset.Fields returns a Fields object.
  • Appropriately name everything the code must interact with: if a rounded rectangle shape is attached to a DoSomething macro, the default “Rounded Rectangle 1” name should be changed to “DoSomethingButton” or something that tells us about its purpose. This includes all controls on a UserForm designer, too. CommandButton12 is useless; SearchButton is much better. Consider also naming the controls that don’t necessarily interact with code, too: future code might, and the author of that future code will appreciate that the bottom panel is named BottomPanel and not Label34.

Renaming

Naming is hard enough, renaming things should be easy. With Rubberduck’s Rename refactoring (Ctrl+Shift+R) you can safely rename any identifier once, and all references to that identifier automatically get updated. Without a refactoring tool, renaming a form control can only be done from the Properties toolwindow (F4), and doing this instantly breaks any event handlers for it; renaming a variable by hand can be tedious, and renaming a single-letter variable like a or i with a local-scope find/replace (Ctrl+H) can get funny if the scope has any comments. Rubberduck knows the exact location of every reference to every identifier in your project, so if you have a module with two procedures that each declare a localThing, when you rename the local variable localThing in the first procedure, you’re not going to be affecting the localThing in the other procedure. But if you rename CommandButton1 to OkButton, then CommandButton1_Click() becomes OkButton_Click().

Parameters & Arguments

  • Prefer passing values as parameters instead of bumping the scope of a variable to module-level, or instead of declaring global variables.
  • Pass parameters ByVal whenever possible.
    • Arrays and User-Defined Type structures cannot and should not be passed by value.
    • Objects are never passed anywhere no matter the modifier: it’s only ever (ByVal: a copy of) a pointer that gets passed around – and most of the time the intent of the author is to pass that pointer by value. A pointer is simply a 32-bit or 64-bit integer value, depending on the bitness of the process; passing that pointer ByRef (explicitly or not) leaves more opportunities for programming errors.
  • Use an explicit ByRef modifier whenever passing parameters by reference.
  • Consider specifying an out prefix to name ByRef return parameters.
    • Consider using named arguments for out-prefixed ByRef return parameters.

Comments

  • Use the single quote ' character to denote a comment.
  • Avoid line-continuing comments; use single quotes at position 1 of each line instead.
  • Consider having a @ModuleDescription annotation at the top of each module.
  • Consider having a @Description annotation for each Public member of a module.
  • Remove comments that describe what an instruction does, replace with comments that explain why an instruction needs to do what it does.
  • Remove comments that summarize what a block of code does; replace with a call to a new procedure with a nice descriptive name.
  • Avoid cluttering a module with banner comments that state the obvious. We know they’re variables, or properties, or public methods: no need for a huge green comment banner to tell us.
  • Avoid cluttering a procedure scope with banner comments that split up the different responsibilities of a procedure: the procedure is doing too many things, split it up and appropriately name the new procedure instead.

Variables

  • Declare all variables, always. Option Explicit should be enabled at all times.
  • Declare an explicit data type, always. If you mean As Variant, make it say As Variant.
  • Consider using a Variant to pass arrays between scopes, instead of typed arrays (e.g. String()).
    • Pluralize these identifier names: it signals a plurality of elements/items much more elegantly than Pirate Notation (arr*) does.
  • Avoid Public fields in class modules; encapsulate them with a Property instead.
  • Consider using a backing user-defined Private Type structure for the backing fields of class properties; doing so eliminates the need for a prefixing scheme, lets a property be named exactly as per its corresponding backing field, and cleans up the locals toolwindow by grouping the fields under a single module variable.
  • Limit the scope of variables as much as possible. Prefer passing parameters and keeping the value in local scope over promoting the variable to a larger scope.
  • Declare variables where you’re using them, as you need them. You should never need to scroll anywhere to see the declaration of a variable you’re looking at.

Late Binding

Late binding has precious little to do with CreateObject and whether or not a library is referenced. In fact, late binding happens implicitly rather easily, and way too often. Strive to remain in early-bound realm all the time: when the compiler / IntelliSense doesn’t know what you’re doing, you’re on your own, and even Option Explicit can’t save you from a typo (and error 438).

  • Avoid making a member call against Object or Variant. If a compile-time type exists that’s usable with that object, a local variable of that data type should be assigned (Set) the Object reference and the member call made early-bound against this local variable.
    • Taking an object presenting one interface and assigning it to another data type is called “casting”.
  • Of course explicit late binding is OK (As Object, no library reference, create objects with CreateObject instead of the New operator). Late binding is very useful and has many legitimate uses, but generally not when the object type is accessible at compile-time through a library reference.
  • Avoid the dictionary-access (aka “bang”) operator !, it is late-bound by definition, and makes what’s actually a string literal read like a member name, and any member call chained to it is inevitably late-bound too. Rubberduck can parse and resolve these, but they’re much harder to process than standard method calls.

Explicitness

  • Use explicit modifiers everywhere (Public/Private, ByRef/ByVal).
  • Declare an explicit data type, even (especially!) if it’s Variant.
  • Avoid implicit qualifiers for all member calls: in Excel watch for implicit ActiveSheet references, implicit ActiveWorkbook references, implicit containing worksheet references, and implicit containing workbook references, as they are an extremely frequent source of bugs.
  • Invoke parameterless default members explicitly.
    • Note: some object models define a hidden default member (e.g. Range.[_Default]) that redirects to another member depending on its parameterization. In such cases it’s best to invoke that member directly; for example use Range.Value as appropriate, but the hidden [_Default] member is better off not being invoked at all, for both readability and performance reasons.
  • Invoke parameterized default members implicitly when they are indexers that get a particular item in an object collection, for example the Item property of a Collection. Invoking them explicitly doesn’t hurt, but could be considered rather verbose.
  • Call is not a keyword that needs to be in your program’s vocabulary when you use expressive, descriptive procedure names that imply an action taking place.
  • Consider explicitly qualifying standard module member calls with the project (and module) name, including for standard and referenced libraries, especially in VBA projects that reference multiple object models.

Structured Programming (Procedural)

  • One macro/script per module. Do have it in a module rather than a worksheet’s code-behind.
  • Public procedure first, followed by parameterized Private procedures, in decreasing abstraction level order such that the top reads like a summary and the bottom like boring, small but specific operations.
    • You know it’s done right when you introduce a second macro/module and get to pull the small, low-abstraction, specific operations into Public members of a utility module, and reuse them.
  • Don’t Repeat Yourself (DRY).
  • Consider passing the relevant state to another procedure when entering a block of code. Code is simpler and easier to follow when the body of a loop or a conditional block is pulled into its own scope.
  • Avoid using error handling to control the flow of execution: the best error handling is no error handling at all, because assumptions are checked and things are validated. For example instead of opening a file from a parameter value, first verify that the file exists instead of handling a file not found error… but still handle errors, for any exceptional situations that might occur while accessing the file.
  • When it’s not possible to avoid error handling, consider extracting a Boolean function that swallows the expected error and returns False on failure, to simplify the logic.
  • Handle errors around all file and network I/O.
  • Never trust user inputs to be valid or formatted as expected.

Object Oriented Programming

In VBA/VB6 we get to go further than mere scripting and apply Object-Oriented Programming principles, probably more relevantly so in VB6 and larger VBA projects. For many years it has been drilled into our heads that VBA/VB6 cannot do “real” OOP because it doesn’t support inheritance. The truth is that there is much, much more to OOP than inheritance, and if you want to learn and apply OOP principles in your VBA/VB6 code, you absolutely can, and you absolutely should, and Rubberduck will absolutely help you do that.

  • Adhere to standard OOP best practices, they are general, language-agnostic concepts that couldn’t care less about the capabilities of VBA/VB6:
    • Single Responsibility Principle – each abstraction should be responsible for one thing.
    • Open/Closed Principle – write code that doesn’t need to change unless the purpose of the abstraction itself needs to change.
    • Liskov Substitution Principle – code should run the exact same execution paths regardless of the concrete implementation of a given abstraction.
    • Interface Segregation Principle – keep interfaces small and specialized, avoid a design that constantly needs new members to be added to an interface.
    • Dependency Inversion Principle – depend on abstractions, not concrete implementations.
  • Leverage composition where inheritance would be needed.
  • You cannot have parameterized constructors, but you still can leverage property injection in factory methods to inject instance-level dependencies.
  • Leverage method injection to inject method-level dependencies.
  • Avoid New-ing dependencies in-place, it couples a class with another, which hinders testability; inject the dependencies instead.
    • Use the New keyword in your composition root, as close as possible to an entry point.
    • The Workbook_Open event handler (Excel) is a possible entry point.
    • Macros (Sub procedures invoked from outside the code) are also valid entry points.
    • Let go of the idea that a module must control every last one of its dependencies: let something else deal with creating or dereferencing these objects.
  • Inject an abstract factory when a dependency cannot or should not be created at the composition root, for example if you needed to connect to a database and wish to keep the connection object as short-lived and tightly-scoped as possible.
  • Keep the default instance of a class stateless as much as possible. Actively protect/guard against accidental misuse by throwing a run-time error as necessary.
  • Use standard modules instead of a utility class with a @PredeclaredId, that never gets explicitly instantiated or used as an actual object.

User Interfaces

UI code is inherently object-oriented, and thus a UserForm should be treated as the object it wants to be. The responsibilities of a user interface are simple: display and collect data to/from the user, and/or offer a way to execute commands (which typically consume or otherwise manipulate the data).

  • Avoid working directly with the form’s default instance. New it up instead.
  • Form module / code-behind should be strictly concerned with presentation concerns.
    • Do implement UI logic in form’s code-behind, e.g. enable this control when this command says it can be executed, or show this label when the model isn’t valid, etc.
  • Consider creating a model class to encapsulate the form’s state/data.
    • Expose a read/write property for each editable field on the form.
    • Expose a read-only property for data needed by the controls (e.g. the items of a ListBox).
    • Controls’ Change handlers manipulate the model properties.
    • Expose additional methods and properties as needed for data/input validation.
      • Consider having an IsValid property that returns True when all required values are supplied and valid, False otherwise; use this property to enable or disable the form’s Accept button.
  • Avoid implementing any kind of side-effecting logic in a CommandButton‘s Click handler. A CommandButton should invoke a command, right?
    • In procedural code the command might be a Public Sub procedure in a standard module named after the form, e.g. a SomeDialogCommands module for a SomeDialog form.
    • In OOP the command might be a property-injected instance of a DoSomethingCommand class; the Click handler invokes the command’s Execute method and could pass the model as a parameter.
  • Consider implementing a presenter object that is responsible for owning and displaying the form instance; the Model-View-Presenter UI pattern is well documented, and like everything OOP, its concepts aren’t specific to any language or platform.

Caveat: Microsoft Access Data-Bound UI

VBA projects hosted in Microsoft Access can absolutely use UserForm modules too, but without Rubberduck you need to hunt down the IDE command for it because it’s hidden. Instead, in Access you mostly create Access Forms, which (being document modules owned by the host application) have much more in common with a Worksheet module in Excel than with a UserForm.

The paradigm is different in an Access form, because of data bindings: a data-bound form is inherently coupled with the underlying database storage, and any effort to decouple the UI from the database is working directly against everything Access is trying to make easier for you.

Treating an Access form the way one would treat a worksheet UI in Excel puts you in a bit of a different mindset. Imagine the Battleship worksheet UI implemented as an Access form: the game would be updating game state records in the underlying database, and instead of having code to pull the game state into the UI there would only need to be code to re-query the game state, and the data bindings would take care of updating the actual UI – and then the game could easily become multi-player, with two clients connecting to the database and sharing the same game state.

This is very fundamentally different than how one would go about getting the data into the controls without such data bindings. Binding the UI directly to a data source is perfectly fine when that data source happens to be running in the very same process your VBA code is hosted in: Access’ Rapid Application Development (RAD) approach is perfectly valid in this context, and its global objects and global state make a nice beginner-friendly API to accomplish quite a lot, even with only a minimal understanding of the programming language (and probably a bit of Access-SQL).

If we’re talking about unbound MS-Access forms, then it’s probably worth exploring Model-View-Presenter and Model-View-Controller architectures regardless: in such exploratory OOP scenarios the above recommendations can all hold.

UI Design

I’m not going to pretend to be a guru of UI design, but over the years I’ve come to find myself consistently incorporating the same elements in my modal forms, and it has worked very well for me so here we go turning that into general guidelines.

  • TopPanel is a Label control with a white background that is docked at the top and tall enough to comfortably fit short instructions.
  • BottomPanel is also a Label control, with a dark gray background, docked at the bottom and no more than 32 pixels in height.
  • DialogTitle is another Label control with a bold font, overlapping the TopPanel control.
  • DialogInstructions is another Label control overlapping the TopPanel control.
  • DialogIcon is an Image control for a 16×16 or 24×24 .bmp icon aligned left, at the same Top coordinate as the DialogTitle control.
  • OkButton, CancelButton, CloseButton, ApplyButton would be CommandButton controls overlapping the BottomPanel control, right-aligned.

The actual client area content layout isn’t exactly free-for-all, and I doubt it’s possible to come up with a set of “rules” that can apply universally, but we can try, yeah?

  • Identify each field with a label; align all fields to make it look like an implicit grid.
  • Seek visual balance; ensure a relatively constant margin on all sides of the client area, space things out but not too much. Use Frame controls to group ComboBox options.
  • Avoid making a complex form with too many responsibilities and, inevitably, too many controls. Beyond a certain complexity level, consider making separate forms instead of tabs.
  • Use Segoe UI for a more modern font than MS Sans Serif.
  • Do not bold all the labels.
  • Have a ToolTip string for the label of every field the user must interact with. If a field is required or demands a particular format/pattern, indicate it.
  • Consider toggling the visibility of a 16×16 icon next to (or even inside, right-aligned) input fields, to clearly indicate any data validation errors (have a tooltip string on the image control with the validation error message, e.g. “this field is required”, or “value cannot be greater than 100”).
  • Name. All. The. Things.
  • Use background colors in input controls only to strongly signal something to the user, like a validation error that must be corrected in order to move on. Dark red text over a light pink background makes a very strong statement.
  • Keep a consistent color scheme/palette and style across all of your application’s UI components.

This pretty much concludes the “guidelines” section (although I’ll quite probably be adding more to it), but since discussing unit testing and testability lines up with everything above…

Unit Testing

A unit test is a small, simple procedure that is responsible for 3 things:

  1. Arrange dependencies and set expectations.
  2. Act, by invoking the method or function under test.
  3. Assert that the expected result matches the actual one.

When a unit test runs, Rubberduck tracks Assert.Xxxx method calls and their outcome; if a single Assert call fails, the test fails. Such automated tests are very useful to document the requirements of a particular model class, or the behavior of a given utility function with multiple optional parameters. With enough coverage, tests can actively prevent regression bugs from being inadvertently introduced as the code is maintained and modified: if a tweak breaks a test, you know exactly what functionality you broke, and if all tests are green you know the code is still going to behave as intended.

Have a test module per unit/class you’re testing, and consider naming the test methods following a MethodUnderTest_GivenAbcThenXyz, where MethodUnderTest is the name of the method you’re testing, Abc is a particular condition, and Xyz is the outcome. For tests that expect an error, consider following a MethodUnderTest_GivenAbc_Throws naming pattern. Rubberduck will not warn about underscores in test method names, and these underscores are safe because Rubberduck test modules are standard modules, and unit test naming recommendations usually heavily favor being descriptive over being concise.

What to test?

You want to test each object’s public interface, and treat an object’s private members as implementation details. You do NOT want to test implementation details. For example if a class’ default interface only exposes a handful of Property Get members and a Create factory method that performs property-injection and a handful of properties, then there should be tests that validate that each of the parameters of the Create method correspond to an injected property. If one of the parameters isn’t allowed to be Nothing, then there should be a guard clause in the Create method for it, and a unit test that ensures a specific error is being raised when the Create method is invoked with Nothing for that parameter.

Below is one such simple example, where we have 2 properties and a method; note how tests for the private InjectDependencies function would be redundant if the public Create function is already covered – the InjectDependencies function is an implementation detail of the Create function:

'@PredeclaredId
Option Explicit
Implements IClass1
Private Type TState
    SomeValue As String
    SomeDependency As Object
End Type
Private This As TState
Public Function Create(ByVal SomeValue As String, ByVal SomeDependency As Object) As IClass1
    If SomeValue = vbNullString Then Err.Raise 5
    If SomeDependency Is Nothing Then Err.Raise 5
    Dim Result As Class1
    Set Result = New Class1
    InjectProperties Result, SomeValue, SomeDependency
    Set Create = Result
End Function
Private Sub InjectProperties(ByVal Instance As Class1, ByVal SomeValue As String, ByVal SomeDependency As Object)
    Instance.SomeValue = SomeValue
    Set Instance.SomeDependency = SomeDependency
End Sub
Public Property Get SomeValue() As String
    SomeValue = This.SomeValue
End Property
Public Property Let SomeValue(ByVal RHS As String)
    This.SomeValue = RHS
End Property
Public Property Get SomeDependency() As Object
    SomeDependency = This.SomeDependency
End Property
Public Property Set SomeDependency(ByVal RHS As Object)
    Set This.SomeDependency = RHS
End Property
Private Property Get IClass1_SomeValue() As String
    IClass1_SomeValue = This.SomeValue
End Property
Private Property Get IClass1_SomeDependency() As Object
    IClass1_SomeDependency = This.SomeDependency
End Property

Note: the property injection mechanism doesn’t need a Property Get member on the Class1 interface, however not exposing a Property Get member for a property that has a Property Let (and/or Property Set) procedure, would leave the property as write-only on the Class1 interface. Write-only properties would be flagged as a design smell, so there’s a conundrum here: either we expose a Property Get that nothing is calling (except unit tests, perhaps), or we expose a write-only property (with a comment that explains its property injection purpose). There is no right or wrong, only a consistent design matters.

If we were to write unit tests for this class, we would need at least:

  • One test that invokes Class1.Create with an "" empty string for the first argument and fails if error 5 isn’t raised by the procedure call.
  • One test that invokes Class1.Create with Nothing for the second argument and fails if error 5 isn’t raised by the procedure call.
  • One test that invokes Class1.Create with valid arguments and fails if the returned object is Nothing.
  • One test that invokes Class1.Create with valid arguments and fails if the Class1.SomeValue property doesn’t return the value of the first argument.
  • One test that invokes Class1.Create with valid arguments and fails if the Class1.SomeDependency property doesn’t return the very same object reference as was passed for the second argument.
  • One test that invokes Class1.Create with valid arguments and fails if the IClass1.SomeValue property doesn’t return the same value as Class1.SomeValue does.
  • One test that invokes Class1.Create with valid arguments and fails if the IClass1.SomeDependency property doesn’t return the same object reference as Class1.SomeDependency does.

Obviously that’s just a simplified example, but it does perfectly illustrate the notion that the answer to “what to test?” is simply “every single execution path”… of every public member (private members are implementation details that are invoked from the public members; if they specifically need tests, then they deserve to be their own concern-addressing class module).

What is testable?

Without the Property Get members of Class1 and/or IClass1, we wouldn’t be able to test that the Create method is property-injecting SomeValue and SomeDependency, because the object’s internal state is encapsulated (as it should be). Therefore, there’s an implicit assumption that a Property Get member for a property-injected dependency is returning the encapsulated value or reference, and nothing more: by writing tests that rely on that assumption, we are documenting it.

Now SomeDependency might be an instance of another class, and that class might have its own encapsulated state, dependencies, and testable logic. A more meaty Class1 module might have a method that invokes SomeDependency.DoSomething, and the tests for that method would have to be able to assert that SomeDependency.DoSomething has been invoked once.

If Class1 wasn’t property-injecting SomeDependency (for example if SomeDependency was being New‘d it up instead), we would not be able to write such a test, because the outcome of the test might be dependent on a method being called against that dependency.

A simple example would be Class1 newing up a FileSystemObject to iterate the files of a given folder. In such a case, FileSystemObject is a dependency, and if Class1.DoSomething is newing it up directly then every time Class1.DoSomething is called, it’s going to try and iterate the files of a given folder, because that’s what a FileSystemObject does, it hits the file system. And that’s slow. I/O (file, network, …and user) is dependent on so many things that can go wrong for so many reasons, having it interfere with tests is something you want to avoid.

The way to avoid having user, network, and file inputs and outputs interfere with the tests of any method, is to completely let go of the “need” for a method to control any of its dependencies. The method doesn’t need to create a new instance of a FileSystemObject; what it really needs is actually a much simpler any object that’s capable of returning a list of files or file names in a given folder.

So instead of this:

Public Sub DoSomething(ByVal Path As String)
    With CreateObjet("Scripting.FileSystemObject")
        ' gets the Path folder...
        ' iterates all files...
        ' ...
    End With
End Sub

We would do this:

Public Sub DoSomething(ByVal Path As String, ByVal FileProvider As IFileProvider)
    Dim Files As Variant
    Files = FileProvider.GetFiles(Path)
    ' iterates all files...
    ' ...
End Sub

Where IFileProvider would be an interface/class module that might look like this:

Option Explicit
'@Interface
'@Description "Returns an array containing the file names under the specified folder."
Public Function GetFiles(ByVal Path As String) As Variant
End Function

That interface might very well be implemented in a class module named FileProvider that uses a FileSystemObject to return the promised array.

It could also be implemented in another class module, named TestFileProvider, that uses a ParamArray parameter so that unit tests can take control of the IFileProvider dependency and inject (here by method injection) a TestFileProvider instance. The DoSomething method doesn’t need to know where the file names came from, only that it can expect an array of existing, valid file names from IFileProvider.GetFiles(String). If the DoSomething method indeed doesn’t care where the files came from, then it’s adhering to pretty much all OOP design principles, and now a test can be written that fails if DoSomething is doing something wrong – as opposed to a test that might fail if some network drive happens to be dismounted, or works locally when working from home but only with a VPN.

The hard part is obviously identifying the dependencies in the first place. If you’re refactoring a procedural VBA macro, you must determine what the inputs and outputs are, what objects hold the state that’s being altered, and devise a way to abstract them away and inject these dependencies from the calling code – whether that caller is the original entry point macro procedure, or a new unit test.

Mocking

In the above example, the TestFileProvider implementation of the IFileProvider dependency is essentially a test stub: you actually write a separate implementation for the sole purpose of being able to run the code with fake dependencies that don’t incur any file, network, or user I/O. Reusing these stubs in “test” macros that wire up the UI by injecting the test stubs instead of the actual implementations, should result in the application running normally… without hitting any file system or network.

With mocks, you don’t need to write a “test” implementation. Instead, you configure an object provided by a mocking framework to behave as the method/test needs, and the framework implements the mocked interface with an object that can be injected, that verifiably behaves as configured.

Sounds like magic? A lot of it actually is, from a VBA/VB6 standpoint. Many tests in Rubberduck leverage a very popular mocking framework called Moq. What we’re going to be releasing as an experimental feature is not only a COM-visible wrapper around Moq. The fun part is that the Moq methods we need to use are generic methods that take lambda expressions as parameters, so our wrapper needs to expose an API VBA code can use, and then “translate” it into member calls into the Moq API, but because they’re generic methods and the mocked interface is a COM object, we essentially build a .NET type on the fly to match the mocked VBA/COM interface, so that’s what Moq actually mocks: a .NET interface type Rubberduck makes up at run-time from any COM object. Moq uses Castle Windsor under the hood to spawn instances of proxy types – made-up actual objects that actually implement one or more interfaces. Castle Windsor is excellent at what it does; we use CW to automate dependency injection in Rubberduck (a technique dubbed Inversion of Control, where a single container object is responsible for creating all instances of all objects in the application in the composition root; that’s what’s going on while Rubberduck’s splash screen is being displayed).

There is a problem though: CW seems to be caching types with the reasonable but still implicit assumption that the type isn’t going to change at run-time. In our case however, this means mocking a VBA interface once and then modifying that interface (e.g. adding, removing, or reordering members, or changing a member signature in any way) and re-running the test would still be mocking the old interface, as long as the host process lives. This isn’t a problem for mocking a Range or a Worksheet dependency, but VBA user code is being punished here.

Verifiable Invocations

Going back to the IFileProvider example, the GetFiles method could be configured to return a hard-coded array of bogus test strings, and a test could be made to turn green when IFileProvider.GetFiles is invoked with the same specific Path parameter value that was given to Class1.DoSomething. If you were stubbing IFileProvider, you would perhaps increment a counter every time IFileProvider_GetFiles is invoked, and expose that counter with a property that the test could Assert is equal to an expected value. With Moq, you can make a test fail by invoking a Verify method on the mock itself, that verifies whether the specified method was invoked as configured.

A best practice with mocking would be to only setup the minimal amount of members to make the test work, because of the performance overhead: if a mocked interface has 5 methods and 3 properties but the method under test only needs 2 of these methods and 1 of these properties, then it should only setup these. Verification makes mocking a very valuable tool to test behavior that relies on side-effects and state changes.

The best part is that because VBA is COM, then everything is an interface, so if you don’t have an IFileProvider interface but you’re still passing a FileProvider object as a dependency, then you can mock the FileProvider directly and don’t need to introduce any extra “just-for-testing” IFileProvider interface if you don’t already have one.


I’m going to stop here and just publish, otherwise I’ll be editing this post forever. So much is missing…

10 thoughts on “Rubberduck Style Guide”

  1. Great post, as usually.
    Consider using a Variant to pass arrays between scopes, instead of typed arrays (e.g. String()).

    Why?

    Sub Test()
    Dim arr(5) As String
    DoSthWithArr1 arr
    DoSthWithArr2 arr
    End Sub

    Sub DoSthWithArr1(ByRef arr() As String)
    Debug.Print UBound(arr)
    End Sub
    Sub DoSthWithArr2(ByRef arr As Variant)
    Debug.Print UBound(arr)
    End Sub

    Why DoSthWithArr2 is better than DoSthWithArr1?

    Liked by 1 person

    1. Oh, simply because dealing with typed arrays in VBA has been a ridiculously frustrating experience for me, I’ve long ago given up and embraced just systematically using `Variant` for arrays. Ooh and that reminds me of pluralization naming rules I haven’t mentioned anywhere… But anyway yeah absolutely go ahead and use a typed array if it works for you – it does work nicely in many scenarios.
      I just kind of hate that I’ve been late-binding them for so long I’ve actually forgotten what particular scenarios are making them painful!

      Like

  2. It seems like you have enough material to write a book. Most of your guidelines are resumed in one line but could be detailed in longer (and interesting) paragraphs with examples.
    Some have already been explained in previous posts and it’s nice for you to remind them to us.
    Some are too tricky for me and I think I will never wrap my head around them.
    I often find myself reading older posts and rediscover not so obvious tips.
    Thanks for your dedicated work.

    Liked by 1 person

  3. Still trying to grasp as much OOP in VBA as I can. In your statement:

    Avoid New-ing dependencies in-place, it couples a class with another, which hinders testability; inject the dependencies instead.

    Can you show a quick example of each: the bad coupling and the inject dependencies (this is the part I can’t grok)?

    Thanks! Awesome article as always

    Liked by 1 person

    1. +1 that’s the point where I got confused on Dependency injection. I tried reading it multiple times but a example would be a great help! Thanks for awesome article!

      Like

  4. I love style guides – thank you for this. I was particularly interested in your UI design section – I liked your approach to a fresher looking userform. That said I try and experiment with new/different UI elements – basically, as far away as possible from the the standard command buttons and the sunken effect applied [insert control classname here], etc. I was wondering if your UI design section changes at all with the new Windows 11 UI/design language?

    Liked by 1 person

    1. Thanks! I’ve yet to see how Win11 renders the old MSForms controls, but I don’t think it’ll change the approach much. A visually unbalanced design with inconsistent control sizes and appearances, is more annoying than anything else tbh, regardless of the UI framework being used! “Flat” styling can work relatively well with MSForms, but not all controls look nice that way (at least up to Win10), so I just go with the standard “classic” styling, it’s like comfort food: not particularly spicy or funky in any way, doesn’t surprise anyone but can still be amazingly well done =)

      Like

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s