5

I am starting to use Classes in VBA and appreciate some of the fantastic information that is already available on SO.

As far as I can tell, what seems to be lacking is an explanation of what the code in a class should or, as I suspect, should NOT do. For example:

Lets say I have a document and wish to insert / modify a table. In this example I'd like to:

  • check if the table exists
  • if table does not exist:
    • add the table at a specific location
    • add information to the table (i.e. add rows)
  • if table does exist
    • add / delete information to /from the table
    • sort the table according to some criteria

With respect to 'sorting' I imagine that a class module would be well suited to determining the order that information should be put into a table based on some criteria.

But ideally:

  • Should the class module (or a 2nd class module) be used to check and edit the document?

OR

  • Would checking and/or editing be best done using a regular module?

Or doesn't it matter? If there is a preferred way then what are the advantages / disadvantages of each approach?

SlowLearner
  • 3,086
  • 24
  • 54
  • 2
    A `Document`, in *being* a class is a big hint. There's no reason why your class shouldn't interact with **a** document directly. – ThunderFrame Jul 11 '17 at 04:20
  • @ThunderFrame - hmmm... well rightly or wrongly, in my projects almost all the code in my ThisDocument is executed in a regular module called docThisDocument because that ~seems to~ makes MS-Word run better... would that be the hint you're referring to? – SlowLearner Jul 11 '17 at 04:27
  • You use a class where it makes sense to encapsulate some set of functionality/information in a way that makes your task easier to manage. Often a class is modelling a "thing" in your workflow. However, there's no magic there, and it shouldn't make much/any difference to your code's performance. – Tim Williams Jul 11 '17 at 05:02
  • No, the hint is that `ThisDocument` *is* a predeclared instance of a class module. You interact with the document *as a class* by using the methods and properties of the class. *Your* class is free to work with `ThisDocument` (or better yet, with *any* document that is handed to your class). There's no reason to use a procedure in a standard module to interact with the document. – ThunderFrame Jul 11 '17 at 05:04
  • @ThunderFrame - ok, well that's good to know. Everything that I have read about classes suggests that they are a sort of template for an object as opposed to something that should cause modification to the actual document. I suppose principle of single responsibility principle will come in to play and provide guidance somewhere here... – SlowLearner Jul 11 '17 at 05:11
  • @TimWilliams - thanks for your thoughts, one of my problems is that I seem to lack the foresight to know what makes sense :( I have a document with lots of tables, there are different types of tables but for the most part they are all the same (rows of information). One thing that I need to do a lot of is sorting these tables. Sometimes sorting table A means that table B will need to be re-sorted.. and on it goes. I was thinking that, rather than physically sorting tables, I could sort everything in a class then repopulate the tables... – SlowLearner Jul 11 '17 at 05:19
  • You can actually do what you described simply by using a generic procedure that does the sorting and then calling it out every time you need to execute the sorting task, not necessarily via *Class*. And yes it doesn't matter actually where you do it be it in a *Class Module* or *Regular Module* as already pointed out by Tim. Work on where you're most comfortable. – L42 Jul 11 '17 at 06:28
  • @L42 Yes, I actually have something like that at the moment. I want to implement some best practice coding techniques and hopefully make the overall project easier to manage. – SlowLearner Jul 11 '17 at 08:22

1 Answers1

5

First off, kudos for entering the wonderful rabbit hole of OOP!

Short answer: It depends.


(very) long answer:

You'll want to avoid pulling a worksheet [that exists at compile-time] from the Application.Worksheets (or Application.Sheets) collection, and use that sheet's CodeName instead. VBA creates a global-scope object reference for you to use, named after every worksheet's CodeName.

That's how this code gets to compile, with Sheet1 not being declared anywhere:

Option Explicit

Sub Test()
    Debug.Print Sheet1.CodeName
End Sub

The problem with implementing worksheet-specific functionality anywhere other than in that worksheet's code-behind, using that global-scope "free" object variable, is that the separate module is now coupled with that Sheet1 object.

Class module depending on a worksheet. Any worksheet.

You want focused, cohesive modules - high cohesion. And low coupling.

By writing worksheet-specific code in another module (be it a standard or a class module), you're creating a dependency and increasing coupling, which reduces testability - consider this code in Class1:

Public Sub DoSomething()
    With Sheet1
        ' do stuff
    End With
End Sub

Now Class1 can only ever work with Sheet1. This would be better:

Public Sub DoSomething(ByVal sheet As Worksheet)
    With sheet
        ' do stuff
    End With
End Sub

What happened here? Dependency Injection. We have a dependency on a specific sheet, but instead of coding against that specific object, we tell the world "give me any worksheet and I'll do my thing with it". That's at method level.

If a class means to work with a single specific worksheet, and exposes multiple methods that do various things with that worksheet, having a ByVal sheet As Worksheet parameter on every single method doesn't make much sense.

Instead you'll inject it as a property:

Private mSheet As Worksheet

Public Property Get Sheet() As Worksheet
    Set Sheet = mSheet
End Property

Public Property Set Sheet(ByVal value As Worksheet)
    Set mSheet = value
End Property

And now all methods of that class can work with Sheet... the only problem is that the client code consuming that class now needs to remember to Set that Sheet property, otherwise errors can be expected. That's bad design IMO.

One solution could be to push the Dependency Injection Principle a notch further, and actually depend on abstractions; we formalize the interface we want to expose for that class, using another class module that will act as the interface - that IClass1 class doesn't implement anything, it just defines stubs for what's exposed:

'@Interface
Option Explicit

Public Property Get Sheet() As Worksheet
End Property

Public Sub DoSomething()
End Sub

Our Class1 class module can now implement that interface, and if you've been following this far, hopefully I don't lose you here:

NOTE: Module and member attributes are not visible in the VBE. They're represented here with their corresponding Rubberduck annotations.

'@PredeclaredId
'@Exposed
Option Explicit
Implements IClass1

Private mSheet As Worksheet

Public Function Create(ByVal pSheet As Worksheet) As IClass1
    With New Class1
        Set .Sheet = pSheet
        Set Create = .Self
    End With
End Function

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

Private Property Get IClass1_Sheet() As Worksheet
    Set IClass1_Sheet = mSheet
End Property

Private Sub IClass1_DoSomething()
    'implementation goes here
End Sub

This Class1 class module presents two interfaces:

  • Class1 members, accessible from the PredeclaredId instance:
    • Create(ByVal pSheet As Worksheet) As IClass1
    • Self() As IClass1
  • IClass1 members, accessible from the IClass1 interface:
    • Sheet() As Worksheet
    • DoSomething()

Now the calling code can look like this:

Dim foo As IClass1
Set foo = Class1.Create(Sheet1)
Debug.Assert foo.Sheet Is Sheet1
foo.DoSomething

Because it's written against the IClass1 interface, the calling code only "sees" the Sheet and DoSomething members. Because of the VB_PredeclaredId attribute of Class1, the Create function can be accessed via the Class1 default instance, pretty much exactly like Sheet1 gets accessed without creating an instance (UserForm classes have that default instance, too).

This is the factory design pattern: we're using the default instance as a factory whose role is to create and initialize an implementation of the IClass1 interface, which Class1 just so happens to be implementing.

With Class1 completely decoupled from Sheet1, there's absolutely nothing wrong with having Class1 responsible for everything that needs to happen on whatever worksheet it's initialized with.

Coupling is taken care of. Cohesion is another problem entirely: if you find Class1 is growing hair and tentacles and becomes responsible for so many things you don't even know what it was written for anymore, chances are that the Single Responsibility Principle is taking a beating, and that the IClass1 interface has so many unrelated members that the Interface Segregation Principle is also taking a beating, and the reason for that is probably because the interface wasn't designed with the Open/Closed Principle in mind.


The above couldn't be implemented with standard modules. Standard modules don't play quite well with OOP, which means tighter coupling and thus lower testability.


TL;DR:

There isn't one single "right" way to design anything.

  • If your code can deal with being tightly coupled with a specific worksheet, prefer implementing the functionality for that worksheet in that worksheet's code-behind, for better cohesion. Still use specialized objects (classes) for specialized tasks: if your worksheet code-behind is responsible for setting up a database connection, sending a parameterized query over the network, retrieving the results and dumping them into the worksheet, then you're Doing It Wrong™ and now testing that code in isolation, without hitting the database, is impossible.
  • If your code is more complex and can't afford tight coupling with a specific worksheet, or if the worksheet doesn't exist at compile-time, implement the functionality in a class that can work with any worksheet, and have a class that's responsible for the model of that runtime-created sheet.

IMO a standard module should only be used to expose entry points (macros, UDFs, Rubberduck test methods, and with Option Private Module, some common utility functions), and contain fairly little code that merely initializes objects and their dependencies, and then it's classes all the way down.

Mathieu Guindon
  • 69,817
  • 8
  • 107
  • 235
  • Wow - thank you! Yes you did lose me there for a bit - but I think I recovered. I'm actually using MS Word, but appreciate the 'sheet' example. Finally starting to understand coupling and cohesion thanks to your explanation. This has left me with a few questions for clarification... one of which relates to your closing para where you state your opinion that a standard module should only be used to expose entry points (and include macros in a list)... how do you differentiate between a macro and all the rest of the code? – SlowLearner Jul 11 '17 at 22:59
  • @SlowLearner to me a "macro" is a parameterless `Public Sub` procedure in a standard module, invoked through some kind of UI - be it a button on a worksheet, a Ribbon button, a shortcut key, or just the host application's *Macros* dialog. I treat those as *entry points*; the functionality they invoke is never implemented directly in such procedures, because then you have a "God module" that knows and does *everything*... and then you can't write unit tests for any of it, because everything is just one big blob of code. – Mathieu Guindon Jul 11 '17 at 23:28
  • Do you mean behind the scenes like setting a default as described here: http://www.cpearson.com/excel/DefaultMember.aspx ? – SlowLearner Jul 11 '17 at 23:39
  • That's pretty cool :) - Oh just noticed you comment on Macros, somehow missed it earlier, tnx – SlowLearner Jul 11 '17 at 23:47