# DRYing up an initialization pattern

I maintain a VBA process that prints a large number of PDFs. It was awful when I inherited it and I've been making improvements over time, but I'm a bit stuck on this one. There are at least 4 functions with this exact same code used to check to see if the program has been initialized properly. They could all be called individually, or together depending on circumstances.

I do not want to initialize again if I don't have to. Hence the check in every subroutine, but I just can't figure out how to do it any differently. Thoughts? Is there a design pattern to deal with this kind of thing that I don't know of?

Please try to focus on the pattern and not the supporting intitializer. I know there are a few issues there and intend to deal with them soon. I included it in case it was relevant.

Main code:

Private rootDir As String
Private isInit As Boolean
Private mCycle As String

Public Function printPDF_VoidedNewItems()
If Not isInit Then
initialize
End If
If rootDir <> "" And mCycle <> "" Then
If setPrinterToPDF() Then
' logic specific to printing this pdf
End If
End If
End Function

Public Function printPDF_ComsNotFed()
If Not isInit Then
initialize
End If
If rootDir <> "" And mCycle <> "" Then
If setPrinterToPDF() Then
' logic specific to printing this pdf
End If
End If
End Function


Initializer:

Private Sub initialize()
rootDir = folderPicker() & "\"
mCycle = getCycleInput
isInit = True
End Sub

Private Function folderPicker() As String
'*******************************************
' returns directory path to be printed to
'   does not allow multiple selections,
'   so returning the first item in selected
'   items is sufficient.
'
' returns empty string On Error or if the
'   user cancels
'********************************************
On Error GoTo ErrHandler

Dim folderChosen As Integer
Dim fd As Variant
Set fd = Application.FileDialog(msoFileDialogFolderPicker)
With fd
.AllowMultiSelect = False
.Title = "Choose Directory to Print to"
.InitialFileName = "\\server\share\"
.InitialView = msoFileDialogViewSmallIcons
folderChosen = .Show
If folderChosen <> -1 Then
folderPicker = ""
Else
folderPicker = .SelectedItems(1)
End If
End With
Exit Function

ErrExit:
folderPicker = ""
Exit Function
ErrHandler:
MsgBox "Unexpected Error: " & Err.number & vbCrLf & "Source: " & Err.Source & _
"Description: " & Err.Description, vbCritical, "ERROR!"
End Function

Private Function getCycleInput() As String
' Displays a form with a combo box to the user.
' Returns an empty string if canceled or errored
'   Otherwise returns 8 character Cycle from dbo.MAM_ISE_SCHEDULE
On Error GoTo ErrHandler

Dim cpkr As New Form_CyclePicker

'wait for user input
Do Until cpkr.Cancel = True Or cpkr.Confirmed = True 'Do Until user clicks "OK" or "Cancel"
DoEvents 'frees up OS to actually display form and allow user to interact with it
Loop

If cpkr.Cancel Or Not cpkr.Confirmed Then ' not confirmed seems redundant, but certainly can't hurt to check.
' return
getCycleInput = ""
GoTo ExitSub
Else
' return
getCycleInput = cpkr.CycleNumber
End If

ExitSub:
Set cpkr = Nothing
Exit Function
ErrHandler:
MsgBox Err.Description, vbCritical, "Runtime Error: " & Err.number
getCycleInput = ""
Resume ExitSub
End Function

-
Is the main code in a class module? –  Mat's Mug Jul 2 at 15:45
It probably should be (somehow), but no. It's in a regular .bas module. –  RubberDuck Jul 2 at 15:46
I think it would be best to include the parts you replaced with comments, so we can see how much duplication is really going on. –  Mat's Mug Jul 2 at 15:55
I'm already refactoring the duplication inside of them into their own functions and routines @Mat'sMug. I'd like to focus on this pattern. It's the part I don't know how to deal with. –  RubberDuck Jul 2 at 15:58

your Main code is split into two functions right now, which only differ in the logic inside the nested if statement, you should do the if statement and initializing outside the function and just

If Not isInit Then
initialize
End If
If rootDir <> "" And mCycle <> "" Then
If setPrinterToPDF() Then
' Print VoidedNewItems
' Print ComsNotFed
End If
End If


make the functions the guts in the middle and then call them inside one if statement like above.

Public Function printPDF_VoidedNewItems()
' logic specific to printing this pdf
End Function


then call the function like this.

If Not isInit Then
initialize
End If
If rootDir <> "" And mCycle <> "" Then
If setPrinterToPDF() Then
printPDF_VoidedNewItems()
printPDF_ComsNotFed()
End If
End If


that way you are only doing one check on everything instead of two when you call these functions

If Not isInit Then
initialize
End If
If rootDir <> "" And mCycle <> "" And setPrinterToPDF() Then
printPDF_VoidedNewItems()
printPDF_ComsNotFed()
End If


they are all getting checked the same might as well have all the checks in one place.

-
That is a good idea, but how does the function tell which one to actually print now? –  Mat's Mug Jul 2 at 16:00
But I can call them independently as well... Maybe I could add a Enum ReportType and a parameter to PrintPDF(type as ReportType")?? –  RubberDuck Jul 2 at 16:01
@ckuhn203 that is a good idea –  Malachi Jul 2 at 16:02
alternatively UBound(Filter(Array(rootDir, mCycle), vbNullString)) > -1 –  me how Jul 29 at 10:13
@mehow, I am a little lost here could you please clarify what you are saying a little bit, it's been a while since I wrote this review... –  Malachi Jul 29 at 13:05

### Down with the underscore

I find the naming of printPdf_xxxx functions confusing, because if that code is/were in a class module they should be private, and the class would say Implements printPdf, where printPdf would be a class with VoidedNewItems and ComsNotFed members... or it could be that there's a private field called printPdf that's declared WithEvents and the functions are event handlers made public.

Avoid underscores in procedure & function (/method) names, the language has a meaning for identifiers with an underscore.

### Initializing once

If you scratch that code and implement it in a class instead, and run initialisation in the... Class_Initialize method, which can be thought of as a parameterless constructor that runs when you create the object. Not too crazy about spawning a UI prompt in that handler though, but then the methods wouldn't have to worry about whether or not Initialize has executed.

-
So, if I implement this in a class, I would know already that it's initialized, so there'd be no need to check at all. Nice. Thanks. I should have thought of that. –  RubberDuck Jul 2 at 16:14
Yes. The not-so-great part of it is that you now have the side effect of a UI prompt with every new instantiation of the class though. –  Mat's Mug Jul 2 at 16:16