If…

Rob noticed something in the VBA code I posted the other day to look for that Excel calculation error (or presentation error depending on your view). I always put an Else clause even if its empty. Style? Quality? Habit?

I’ll stick my neck out a little here and say I think which ever works best for you is probably the ‘best’ for you. I personally prefer my way and can justify it (to myself anyway!), but I wouldn’t impose it on another dev. I’ve worked with lots of other peoples code and generally its not that hard to switch, as long as the main things are ok (option explicit, reasonable procedure length, sensible names etc).

Anyway heres my ‘If’ style and why:

If TheNormalCase then
' 'do the normal processing
Else
' 'the non-normal or error proc, or just a comment if no valid else process
End If

I always put the normal/usual/most common case first, all the exception stuff then drops to the lower part of the proc. I only do 3 or 4 levels of nesting max, beyond that I break out into a new procedure. I always put an else (I type the 3 lines together then arrow back up to fill out the logic) and I always consider what to do in the non normal case. If there is nothing to do I just put a comment of why there is no else code. Why bother? Well in Code Complete Steve McConnell discusses some research where missing else clauses was a major cause of bugs. Its his recommendation to put the normal case first too. (I try to do that in IF formulas too).

I rarely use goto but I do if I think it makes the code clearer, or faster if thats critical (and of course error handling).
I also rarely use the one line if statement like

If bOk then doSummat
If not blah than exit sub/function

Simply because there is so often an else to catch, or some tidying up to do.

I’ll post more about other elements of coding style, but I think a lot of it is just that -Style.

Whats your If style? and what are your thought processes around it?

Cheers

Simon

Advertisements

16 Responses to “If…”

  1. MikeC Says:

    Morning Simon,

    Personally, I also always use an If x Then… Else… End If. Never ever ever a 1-line If. Don’t like ’em, they just never look “finished”.

    However, I don’t always put it in the order of

    If (normal case) Then
    ‘Normal Routine
    Else
    ‘Exception Routine
    End If

    Instead, I generally put the more complex scenario in the first part, so:

    If (more complex condition) Then
    ‘More complex routine
    Else
    ‘Less complex routine OR ‘comment do nothing
    End If

    An example of this would be when coursing through a ream of data looking for “standard” errors, and correcting these errors when found – e.g. converting all entries of a certain sort to UCase to avoid having to specify UCase(string) on every occasion later in the code, and trimming leading/trailing “spaces”.

    e.g.
    If Selection UCase(Selection) Then
    Selection = UCase(Trim(Selection))
    Else
    Selection = Trim(Selection)
    End If

    Normally, the first (more complex part) would be a lot more, and the second (less complex) part would be in the region of 0-10 lines, and they wouldn’t be so similar to each other. But this was the best my poor brain could come up with at this time in a morning after 3 hours sleep (thanks dog!!) without digging out a workbook code to copy in… I’m sure the point comes across!

    Generally the only time I use a GoTo is in error trapping. If there are multiple areas within a sub where a “generic” error can be found, and these all require a generic error message (esp in early testing, where they all generate a “Error at Part x – do not remove this message, shout Mike to come over”) so we would see a large number of:

    If (error found) Then
    GoTo ErrTrap
    Else
    ‘continue
    End If

    (or On Error. But we’re talking about If’s)

    And these would be replaced by specific error traps before completion so that an error would actually be vaguely helpful.

    (I do plan to insert a few Vista-style “Fatal Error. Continuing will cause an explosion. Really, I mean it. Continue? “)

    Cheers,
    MikeC

  2. MikeC Says:

    And one of these days, I’ll remember that “greater” “less” and “not equal” signs just don’t work!!!
    *sigh*

  3. Jon Peltier Says:

    I’ve read Code Complete, and follow a lot of Steve McConnell’s guidelines.

    The empty Else in an If or Select Case serves as additional documentation, reminding me that (a) there might be somethign else, and (b) I at least considered it enough to leave it there. It also makes it easier to add the else code, since the structure is already in place.

    Like Simon, when I do an If, even before I type the condition, I put End If and (sometimes) Else. Same with Select Case/End Select, With/End With, For/Next. It’s easier to remember to close all the blocks if you do it when you create them.

    I often will use a Select Case rather than If Then, because I find it easier to add more sub items. Also, it’s easier the do “Case a, b, c” than to do “If a Or b Or c Then”, and I think it’s easier to read.

    I don’t use the one-line “If something Then whatever”, because breaking it over several lines makes it easier to insert lines, and also makes it easier to read. For the same reason, I never concatenate lines of code with a colon.

    GoTo is for error trapping, and for branching to the ExitSub stuff at the end. Rather than this, I usually prefer to structure my way to the end:

    If something Then
    ‘Long block of code
    Else
    ‘ do nothing
    End If
    ExitSub:

    But these rules are only guidelines, because sometimes I do something else. I often get halfway there and wish I’d followed the guideline, and if I’m fixing someone else’s code (like a client gone astray), I wish they’d heard of any guidelines.

  4. Ross Says:

    I hardly ever put an “else” if I don’t need it, some times I might and add a comment if I think it will help me figure out why I put the if in the first place!.
    I never use one line ifs, as that many keywords should ever be so close to each other, nor do use iif – don’t they have to evaluate both conditions?

    I tend not to use select case that much – I’m not sure why, but I just done like them -, and I’ve read that else if’s are quicker. Not sure that’s true, but I bet it is because you just can’t trust those select case statements!

    There’s a copy of code complete (v1) downstairs where I work, I’ve flicked through a couple of pages and it looks great, I’ve even thought about buying it, but sadly I spent all this years book allowance on “Bluff your way in nightclub C++”, which I can heartly recommmed.

  5. Biggus Dickus Says:

    Simon:

    This is one of the reasons I’m not an MVP.

    This thread came out of a comment in another thread about something importanat (testing for bugs in Excel ’07).

    I have found that most often when you post something to answer a question or make a point, the entire point of the info gets lost in this kind of discussion about syntax or “this is how “I” do that”…. implying that the original writer is not qualified to answer the question because he does not do “pure” code. Aaaaaagggghhh……

    I know you guys aren’t being mean here because you are obviously genuinely friends, but on the other hand “Who gives a shit ;-) ” …….. ?

    Dick
    p.s. I use everything all ways depending on how the spirit moves me (Empty Elses, No Elses, Select cases, Do Loops, etc.,etc.) I used to go to seminars and watch Ken Getz demonstrate the speed differences between IF-Then-Elses and Select Statements when run a zillion times until my head exploded. Who cares now at these speeds (??) maybe that’s the wrong attitude though (?)).

  6. MikeC Says:

    Dick – I give a shit!
    I’ve kinda picked up what I know about using Excel “on the job”, have had no formal training etc, and less than 3 years experience in using it above a very basic level. I’m still learning a lot of the time. Seeing how people who are vastly more experienced than I am (like the majority of posting users here, and on DDOE, or at MVP sites etc) do things is really valuable to me, and this is how I pick up a lot of best practice. Plus it’s interesting to see how other people approach problems.
    I sometimes post how I do things because in the past people have kindly picked up on items I could improve on, and suggested alternates that would work better.

    Cheers,
    MikeC

    PS: you think the point gets lost here? Have you been to Worse Than Failure lately? Sheesh!!! =;-)

  7. Dennis Wallentin Says:

    One of the things I really like about .NET is that we can use the Try – Catch – Finally block approach. It gives a nice structure and it’s easy to maintain.

    However, the best error trapping I have ever seen is the following:
    On Error GoTo Hell

    As for If, Else and/or Select Case or ElseIf or And or Or I believe it’s important to support an easy maintenance of the code.

    Kind regards,
    Dennis

  8. Ross Says:

    >IF-Then-Elses and Select Statements when run a zillion times until my head exploded. Who cares now at these speeds (??) maybe that’s the wrong attitude though (?)).

    Maybe it would make a difreance sometimes, but yeah i dont really care, i just use the one i like bestest.

    >One of the things I really like about .NET is that we can use the Try – Catch – Finally block approach. It gives a nice structure and it’s easy to maintain.

    Dam, i though i said that – I ment too, mush have missed it out! Yeah I agree, the try catch, finally is really nice, the err.des are much better too.

  9. Charlie Says:

    I prefer more compact code – so I can see more of the relevant immediate content without having the scroll. As such I do not add else’s that do not do anything. I like the idea of proving that you have thought about the else, but readability which comes with compactness (for me) takes precedence. And I occasionally use single line ifs and IIFs (despite the performance hit) – they make the code easier to understand if the details are handled as details, and not more indentations and more scrolling. Personal preferences to be sure.

    –Charlie

  10. Biggus Dickus Says:

    MikeC:

    I understand what you’re saying. I guess I have the advantage of doing this technology since Excel 3 (and Lotus 1-2-3 1A in fact – ouch) so I don’t think in those terms and maybe I should. Me bad.

    Dick
    p.s. what’s Worse Than Failure ?

  11. Harlan Grove Says:

    The best approach is ‘whatever’s clearest’. Sometimes that means including do-nothing bits, other times tersest is clearest. Depends on who else would be maintaining the code, and determining what’s clearest is an empirical rather than philosophical exercise: what coding style is misunderstood least frequently.

    The only rule I have w/respect to BASIC code is that anyone putting multiple statements on one line using : to separate them should be shot. And, not quite a rule, I usually put a CleanUp label near the end of my procedures, and I use Goto CleanUp rather than Exit {Sub|Function}.

    The problem I have with Simon’s explanation for his approach is that 1-line IFs typically handle unusual states while the common state requires no action. In such cases,

    If state99timesoutof100 Then
    ‘situation normal: do nothing special
    Else
    specialcircumstancecode
    End If

    reduces clarity, at least to me.

  12. Nick Hebb Says:

    I’m more likely not to use an “Else” if it’s not needed. For some simple assignments (boolean, integer, or long), if I think the default case will happen 99% of the time I will create a construct like:

    bFlag = True
    If RareCondition Then
    bFlag = False
    End if

    I will also use colons for initializing some variables. In fact I do it all the time for error trapping code, e.g.:

    On Error GoTo ErrorHandler
    Dim sSub as String: sSub = “SubName”

    Where sSub gets logged by the ErrorHandler. This arrangement lets me use MZTools to insert the error handling when I first create the Sub or Function, then I can add the remaining Dim’s below without having to split the sSub declaration and assignment.

    Also, I will occasionally use single line If’s at the start of a Sub to test for an exit condition.

    I use Exit For occasionally as well – which is just another form of GoTo.

  13. Simon Says:

    Nick, I like that use of : its less clumsy than using 2 lines – I tend to have a dim section, then a setup section, I’ll try your approach next time I think. In C languages I usually do int x = 0; your approach looks to be the nearest thing in Basic. Best wear a bullet proof vest if Harlan is around though.

    Harlan I never have an empty If, probably I would Not it and comment the empty Else. Now that sounds a bit hacky, so maybe I would/should use a 1 liner – I do use them, it just doesn’t seem to be that often.

    Charlie – good point on compactness.

    Dennis/Ross yeah try/catch/finally, I almost forgot.

    Jon – client – guidlines – I know what you mean!

    Mike yeah I give a shit too.

    Dick I hope it doesn’t come across like I’m preaching, i’m not. In Excel/VBA there are plenty of beginner resources, but limited at the higher levels. Hence the questions.
    Phrasing it as ‘I’ is not meant to belittle other devs, far from it, sorry if you read it that way (what would be a better approach?).
    If you aren’t interested don’t chip in, or do the ‘don’t be so anal’ thing to bring us back to earth ;-)
    cheers
    Simon

  14. MikeC Says:

    Dick – check it out.

    Worse Than Failure – Curious Perversions in IT Technology

    Everything from truly insane error messages through to hardware storing that makes the brain hurt (server being used to warm up meat pies, anyone?).

    Originally known as “WTF??”

    Cheers,
    Mike

  15. Jon Peltier Says:

    Nick:

    “Dim sSub as String: sSub = “SubName””

    Isn’t this the same as

    Const sSub as String = “SubName”?

    The single line Ifs I use are like yours:

    If something Then Goto ExitSub

    And I use Exit For and Exit Do all the time. They’re a little more controlled than Gotos.

    I think I have learned many of my practices the hard way, on a “whatever’s clearest” approach like Harlan espouses. For me, “Whatever’s clearest” is what will help me six or twelve months later when someone reports a bug and I have to figure out what I may have been thinking when I wrote the code.

  16. Biggus Dickus Says:

    MikeC:

    “Dick – check it out.

    Worse Than Failure – Curious Perversions in IT Technology”

    Thanx – funny stuff – and shockingly familiar.

    Dick

Leave a Reply

Please log in using one of these methods to post your comment:

WordPress.com Logo

You are commenting using your WordPress.com 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 )

Google+ photo

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

Connecting to %s


%d bloggers like this: