You wouldn’t believe how the cold dark winters evening simply fly by here (well at least I managed to get home even though 4″ (100mm for the metricians) of snow fell this afty, the first November neige in Geneva since 1980 allegedly). And its still dumping it down – might have to throw a sickie tomorrow and go sledging with the kids.
Anyway the big argument today was test code.
Should you or should you not put your VBA test code into production?
Should you strip down your project to the absolute minimum clean prod only code?
Or should you leave in the code you used to test your production code? (assuming you are one of the 3 VBA devs worldwide who bothers to test of course)
My vote is to leave it in, and even though I was in the minority at school today that doesn’t mean I’m wrong, yet.
If your test code is crappy and distracting then yep take it out, no probs (in fact take out all the crappy distracting code), but why I think decent test harness code should be left in:
- It helps show what the code is meant to do
- It helps when you are trying to fix something later in production at a users’ desk.
- It shows that someone did some testing sometime
- If you make significant edits to your code you should retest, if that edit was removing what you thought was the test code how will you check you haven’t broken something? add a test harness???
What do you think? what am I missing that these clean code freaks can see? Remember I’m not for a minute suggesting leaving in a load of random junk scattered throughout a project. I’m thinking of separate modules or at least sections with a bunch of meaningful tests that exercise the main functionality of the system in a controlled way.
What do you do?
cheers
Simon
Wednesday, 1st December, 2010 at 12:38 pm |
Despite being borderline OCD on pretty code, if I’ve written a good ‘test harness’ I’ll definitely leave it in. Paranoid code (I know I can see the value but let’s just make sure….) I’ll take out because I can’t read code longer than a para or so. Then there’s a grey area in between.
If clients are particularly good to me, as a freebie I make sure there’s enough there and there’s some doc so that somebody half-way competent can pick it up easily, even if the tech level is only ‘stone knives and bearskins’ at the time.
Wednesday, 1st December, 2010 at 12:47 pm |
Hi Simon,
I am just in process of delivering project to some client. I always have my working version of application with quite thorougly commented code (where needed) and some testing routines and at the end the version without comments and all the other junk. So, when I am satisfied with my local testing I clean the code with “VBA Code Cleaner” and deliver it to the client. In case of any errors or some requests for additional functionality that has to be implemented I do that in my version. Again I clean the code and deliver it to the client. I never do corrections in client’s version because I would be quite lost with code not indented and without comments.
Beside I always protect VBA code and hide some defined formulas (names).
Do you protect it and do customers ask you to give them a code in some form upon delivery?
Cheers,
Drazen
Wednesday, 1st December, 2010 at 5:29 pm |
I rarely remove test code – although I do try to add it such that it is hived off somewhere and not cluttering up the main code. I can see the logic to removing it, but not sure it is worth the effort – besides, the client paid for the test code, so shouldn’t they get it
Wednesday, 1st December, 2010 at 6:32 pm |
And do they pay for my time removing it? Nope – it stays unless I just feel like it…
Saturday, 4th December, 2010 at 7:22 am |
The comments have pretty well covered the pros and cons.
If you’re the only one who will ever look at the code you can do what you like and only suffer if the gap before you look at it again means you’re unfamliar with it.
1) Is the test code clearly identified as such? Or does it look like real code and a reviewer has to discover it’s not? I create mine as Sub TestThisThing , just above the ThisThing function, and when ready to ship move it to a Tests module that I may or may not delete depending on the relationship with the client/user.
2) Is it at the same current level as the main app or does it test old stuff? ie is a fail good or bad news?
3) If the work is for a external client does the test code give them something they might use, in which case it’s not test code any more but part of validation which you now have to maintain as part of the contract.
It may also give them a hook into some functionality you don’t want them to have (“Oh, I just found this macro does this neat thing…”)
Similar arguments apply to test formulas – if they are littered with #REF because test data was deleted, it’s a distraction.
Monday, 6th December, 2010 at 8:37 pm |
‘Beside I always protect VBA’
Sorry, Drazen – there is no real protection for Excel VBA code. About 60 seconds with a hex editor and the lock is picked.
I usually leave the test harnesses in but will comment them out.
As an aside I also use MZTools to add in ;ine numbering. All the error handling writes to a log file with the user id, module/class, method and line number. I’ve found it a huge timer saver in bug/issue racking.
Thursday, 16th December, 2010 at 10:50 am |
Marcus, I know that, the protection is there just to prevent changing the code by accident.
BTW do customers ask you to give them code (unprotected, printed…) or how is this usually regulated?
Your idea of writing all error handling including line numbers to a log file is great! :-) Yesterday I was just thinking to implement something like that in my code, what a coincidence…probably because I delivered application to client.
Do you mind to share yours?
Friday, 17th December, 2010 at 5:36 pm |
Hi Drazen – here’s an old version. Now days I also include the machine name in the log and have the whole thing wrapped in a class.
Every class/module/form has a private constant (cstrMod) which assigns the name of that document. Each routine/method has a constant (cstrProc) which identifies the error source. You call the log as such:
ErrorLog cstrMod, cstrProc, Err.Number, Err.Description, Erl
Erl is the little documented method to obtain the line number where the error occurred. Note that this means that each line of VBA code is actually numbered. I use MZTools to do this. This gets saved as a CSV so opening in Excel is easy.
Note also the the routine employs two external calls. One is to a custom class called SystemLibrary from which I retrieve system details (User Id, computer name, temp folder etc). The other is to a function called FileExists.
Being able to pinpoint the exact line an error occurred saves a huge amount of time. You also want to ensure that every non-trivial routine has error handling not that the error has percolated up the call stack and you’re tracking a red herring.
Most clients want access to the code, although I always keep a copy of the code. I never print it, just unprotected (actually I just supply them the password).
Public Sub ErrorLog(ByVal Module As String, ByVal Routine As String, ByVal _
ErrorNum As String, ByVal ErrorMsg As String, ByVal ErrLine)
If Not DebugMode Then On Error GoTo ErrorHandler
Dim intFileNum As Integer
Dim strFile As String
Dim strPath As String
Dim strLine As String
Dim blnInsertHeader As Boolean
Dim S As SystemLibrary
Set S = New SystemLibrary
Const cstrHeader As String = _
“LogonId,DateStamp,Module,Routine,Line,ErrorNum,ErrorDesc”
strFile = “ErrorLog.csv”
‘/strPath = App.Path
strPath = ThisWorkbook.path
If Right(strPath, 1) “\” Then
strPath = strPath & “\”
End If
‘ Set flag to insert file header as first row if a
‘ process log doesn’t currently exist
If Not FileExists(strPath & strFile, vbNormal) Then
blnInsertHeader = True
End If
intFileNum = FreeFile
Open strPath & strFile For Append As #intFileNum
‘ Insert the header row if required
If blnInsertHeader = True Then
Print #intFileNum, cstrHeader
End If
‘ replace any commas or non-printing characters in the Err description
ErrorMsg = Replace(ErrorMsg, “,”, “;”)
ErrorMsg = Replace(ErrorMsg, vbCr, ” “)
ErrorMsg = Replace(ErrorMsg, vbLf, ” “)
ErrorMsg = Replace(ErrorMsg, vbCrLf, ” “)
ErrorMsg = Replace(ErrorMsg, vbNewLine, ” “)
‘ Build string to write to file
strLine = S.UserName & “,” & _
Format(Now(), “DD-MMM-YYYY HH:MM:SS”) & “,” & _
Module & “,” & _
Routine & “,” & _
ErrLine & “,” & _
ErrorNum & “,” & _
ErrorMsg
Print #intFileNum, strLine
Close #intFileNum
ExitFunction:
Set S = Nothing
Exit Sub
ErrorHandler:
Resume ExitFunction
End Sub
Wednesday, 22nd December, 2010 at 3:24 pm |
Hi Marcus, thank you very much for the explanation.
I already found some stuff about Erl, so I understand that part, but what I didn’t get are these constants and the way you use them?
In other words, in error handling code I want to determine where the error happened, in which module and in which procedure.
So, how can I determine that? Do I need to setup constants in every procedure? How can I get the names of module and procedure?
Do I need to set references to Microsoft Visual Basic For Applications Extensibility 5.3 in order to work?
Is it possible to send me the code to my e-mail drazenh at gmail dot com? I will appreciate it.
Thanks,
Drazen
Thursday, 23rd December, 2010 at 1:43 pm |
@Drazen,
You cannot grab the modules and procedures automatically, so you do need to create the values in each procedure and pass these to the rror handler.
There is no need to set a reference.
Thursday, 23rd December, 2010 at 10:00 pm |
Hi Drazen,
Here’s some responses to your questions…
“…Do I need to setup constants in every procedure? …”
Unfortunately the short answer is ‘yes’.
VB/VBA is not as sophisticated as some more modern languages/development environments. Passing an exception object in C# (okay, .Net) contains everything you’d want to know. However, in VB/A to determine the module and routine in which an error was generated requires that you implement some basic plumbing code. The constant at every module and routine does exactly that. Hence when a call to the Error Log routine is made, it gets passed the current module, routine and line number. The first two via constants and the later via Erl.
“Do I need to set references to Microsoft Visual Basic For Applications Extensibility 5.3 in order to work?”
Nope, not at all.
“Is it possible to send me the code to my e-mail”
I’d much prefer to post anything you like here so others who are learned can contribute and those who are learning can benefit. Let me know what you’d like to learn.0
Friday, 24th December, 2010 at 11:02 am |
Thank you Bob and Marcus.
OK, I think I understand now, I’ll have to define constant with the name of module and procedure in every one of them and use them in calls to the error handling procedure?
I thought there might be some more general and easier way…
I’ll try to do it by myself…
All the best to all of you here for Christmas and New Year!
Drazen