Solved

# Good coding practice - nested if's

Posted on 2001-07-18
268 Views
I dislike the look of nested if statements and I have been recently using a nested loop.
This also seems to me to be a bit of a bodge so I am wondering what would be the 'correct' way of doing this.

I realise that there is probably no one correct answer so the points will go to the suggestion that I like the best (or that makes most sense).

The procedure reads a line from a file and deals with the content of the line.
Then reads the next line and continues until the end of the file.

nested if:

Private Sub Command1_Click()
Dim lin As String
Dim fnum As Integer
fnum = FreeFile
Open filename For Input As #fnum
Do Until EOF(fnum)
Line Input #fnum, lin
If condition1 Then
If condition2 Then
If condition3 Then
If condition4 Then
If condition5 Then
DoStuff lin
End If
End If
End If
End If
End If
Loop
Close (fnum)
End Sub

nested loop:

Private Sub Command2_Click()
Dim lin As String
Dim ra() As String
fnum = FreeFile
Open filename For Input As #fnum
Do Until EOF(fnum)
Do While True
Line Input #1, lin
If Not condition1 Then Exit Do
If Not condition2 Then Exit Do
If Not condition3 Then Exit Do
If Not condition4 Then Exit Do
If Not condition5 Then Exit Do
DoStuff lin
Loop
Loop
Close (fnum)
End Sub

0
• 10
• 9
• 8
• +7

LVL 6

Expert Comment

ID: 6295435
If condition1 Then
If condition2 Then
If condition3 Then
If condition4 Then
If condition5 Then
DoStuff lin
End If
End If
End If
End If
End If

can become:

If (condition1) and (condition2) and (condition3) and (condition4) and (condition5) Then
doStuff lin
End if
0

LVL 5

Expert Comment

ID: 6295443
Depends on exactly what Condition1, etc. are. Sometimes, you can do this:

If condition1 And condition2 And condition3 And condition4 And condition5 Then
0

LVL 6

Expert Comment

ID: 6295445
and this:

If Not condition1 Then Exit Do
If Not condition2 Then Exit Do
If Not condition3 Then Exit Do
If Not condition4 Then Exit Do
If Not condition5 Then Exit Do

can become:

If Not(Condition1) Or Not(Condition2) Or Not(Condition3) Or Not(Condition4) Or Not(Condition5) Then Exit Do
0

LVL 5

Expert Comment

ID: 6295449
<sigh> Too slow again...
0

LVL 5

Expert Comment

ID: 6295460
<<If Not(Condition1) Or Not(Condition2) Or Not(Condition3) Or Not(Condition4) Or Not(Condition5) Then Exit Do>>

Or this:

If Not(condition1 And condition2 And condition3 And condition4 And condition5) Then Exit Do
0

LVL 7

Expert Comment

ID: 6295461
Depending on exactly what you are doing, a select case may work well. Either do a

Select Case variable
Case 1 ' or whatever
case 2
End select

or, if your conditions are more complex:

Select Case True
Case variable > 2 and blnFlag = True
Case ' etc
End select

Just be aware that if you use the second approach, even if more than one case is true, only the first will be evaluated.

Zaphod.
0

LVL 17

Author Comment

ID: 6295482
The conditions are very likely to be function calls which it may be necessary to carry out in order.
eg. if enough data exists an account may be created,if more data exists then an existing account may be updated.

I'm afraid that I find long lines hard to read/debug as well as the nested if.
0

LVL 5

Accepted Solution

ID: 6295499
I'm not really suggesting this, but I tend to use something like:

Line Input #fnum, lin
If Evaluate(line) Then DoStuff lin

Then put all the nasty stuff in a function called Evaluate or whatever's relevant. It doesn't actually get rid of the problem, but tucks it out of sight...
0

LVL 6

Expert Comment

ID: 6295529
Private Sub Command3_Click()
Dim lin As String
Dim ra() As String

fnum = FreeFile
Open FileName For Input As #fnum

Do While Not lin.EOF
Line Input #1, lin
Select Case True
Case condition1, condition2, condition3, condition4, condition5
Exit Do
Case Else
DoStuff lin
End Select
Loop

Close (fnum)
End Sub
0

LVL 28

Expert Comment

ID: 6295549
>>I'm afraid that I find long lines hard to read/debug as well as the nested if

you can put the conditions on separate lines:

If (condition1 _
And condition2 _
And condition3 _
And condition4 _
And condition 5) Then
0

LVL 5

Expert Comment

ID: 6295555
Good point, AzraSound! I always forget about that, since I personally hate it...
0

LVL 22

Expert Comment

ID: 6295694
Sometimes it's worthwhile to relegate such ugly things to the back crevices of another routine as seen below.  Obviously the conditions may need to be changed depending on the expression's variables.

Private Sub Command1_Click()
Dim lin As String
Dim fnum As Integer
fnum = FreeFile
Open filename For Input As #fnum
Do Until EOF(fnum)
Line Input #fnum, lin
TryToDoStuff
Loop
Close (fnum)
End Sub

' Shove this off in a corner of the module
Private Sub TryToDoStuff(lin as String)
If condition1 Then
If condition2 Then
If condition3 Then
If condition4 Then
If condition5 Then
DoStuff lin
End If
End If
End If
End If
End If
End Sub
0

LVL 1

Expert Comment

ID: 6295726
i don't think putting all teh condition checks into one long statement is very efficient,  like above...
If (condition1 _
And condition2 _
And condition3 _
And condition4 _
And condition 5) Then

as teh function has to evaluate everyone before it can make a decision where as with the nestled if statements the code will run faster cos if the first condition isn't met tehn it exits straight away and doesn't go on checking at the other conditions as above... i remember reading a long article by some guru about writing l33t fast code and this was his explanation which i fully agree with and i've tested it... i think it was from the 101 tips for vb from the vb programmers journal 2000...
so i think that your first one would be the fastest.. what type of conditions are you checking perhaps we coudl clean those up???

0

LVL 1

Expert Comment

ID: 6295730
i don't think putting all teh condition checks into one long statement is very efficient,  like above...
If (condition1 _
And condition2 _
And condition3 _
And condition4 _
And condition 5) Then

as teh function has to evaluate everyone before it can make a decision where as with the nestled if statements the code will run faster cos if the first condition isn't met tehn it exits straight away and doesn't go on checking at the other conditions as above... i remember reading a long article by some guru about writing l33t fast code and this was his explanation which i fully agree with and i've tested it... i think it was from the 101 tips for vb from the vb programmers journal 2000...
so i think that your first one would be the fastest.. what type of conditions are you checking perhaps we coudl clean those up???

0

LVL 22

Expert Comment

ID: 6295773
"the function has to evaluate everyone before it can make a decision"

This is true in the current versions of VB.  The VB.Net documentation indicates that this will be fixed for the .net version.
0

LVL 15

Expert Comment

ID: 6296503
my \$0.02  :-)

- This piece doesn't need "End If"
- Lines are not long
- Code is efficient (if condition1 fails, other conditions are not evaluated)
- There is no need for fake Do...Loop

If condition1 Then _
If condition2 Then _
If condition3 Then _
If condition4 Then _
If condition5 Then _
DoStuff lin: MsgBox "Hello"
0

LVL 1

Expert Comment

ID: 6296957
Select Case False
Case Condition1, Condition2, condition3, Condition4, Condition5
'Do nothing
Case Else
'do ur stuff here
DoStuff lin
End Select

0

LVL 5

Expert Comment

ID: 6296998
ameba, Nice! Very nice!
0

LVL 17

Author Comment

ID: 6297736
ameba, I think that you've hit the nail right on the head about the fake do loop.
In an old version of some Basic that I once used you were able to re-enter a For loop from any part of the block beneath it.

For c = 1 to 100

if not condition1 then next c
if not condition2 then next c
if not condition3 then next c
if not condition4 then next c

Next c

There is probably something very wrong with this as it is not possible in VB, but I think it's something like this that I am looking for.

Reading the code, if the conditions/functions are named carefully then it becomes very clear what state the program is in at each line (perhaps I should say 'statement').
0

LVL 15

Expert Comment

ID: 6297876
RobinD,
>There is probably something very wrong with this

Yes. But after few VB years you get used to it.

"One statement VB lacks is the Continue statement. There are times in the middle of a loop when you want to stop testing and just go to the next iteration in the loop. One can approximate this with nested Ifs but the logic of the nesting can quickly become quite complicated and obscure to follow."

Note that VB has nice function Switch, but it is not very efficient, since all conditions are evaluated.  A workaround to NOT execute all conditions, is to use a flag.  Here is the sample, but, now, those nested IFs don't look so bad.  :-)

Option Explicit
Dim x As Long
Dim blnDone As Boolean

x = 5
If Switch(Condition1, 1 _
, Condition2, 2 _
, Condition3, 3 _
, Condition4, 4 _
, Condition5, 5 _
) = 5 Then

MsgBox "Hello"
End If
End Sub

Private Function Condition1() As Boolean
If blnDone Then Exit Function ' do not execute the 'meat' of the function
If x > 0 Then Condition1 = True
blnDone = True
End Function

Private Function Condition2() As Boolean
If blnDone Then Exit Function ' do not execute the 'meat' of the function
If x > 10 Then Condition2 = True
blnDone = True
End Function

Private Function Condition3() As Boolean
If blnDone Then Exit Function
If x > 20 Then Condition3 = True
blnDone = True
End Function

Private Function Condition4() As Boolean
If blnDone Then Exit Function
If x > 30 Then Condition4 = True
blnDone = True
End Function

Private Function Condition5() As Boolean
If blnDone Then Exit Function
If x > 40 Then Condition5 = True
blnDone = True
End Function

VB has also nice statements "Select Case" and "Choose", but they cannot be used here.
0

LVL 15

Expert Comment

ID: 6297893
OOPS, sorry, Switch cannot be used here.  To strong sun here?  :-)
0

LVL 15

Expert Comment

ID: 6298095
>          If condition1 Then _
>             If condition2 Then _
>             If condition3 Then _
>             If condition4 Then _
>             If condition5 Then _
>             DoStuff lin: MsgBox "Hello"

Although this piece of code satisfies all the conditions, you should not use it:
- it is a bit strange to use multiple "If...Then" in a single line (I am surprised that syntax works) and it's hard to read/understand.
0

LVL 17

Author Comment

ID: 6298143
I do feel like I'm changing the rules as I go along, I hope it doesn't look like it.

I quite liked the look of:
If condition1 Then _
If condition2 Then _
If condition3 Then _
If condition4 Then _
If condition5 Then _
DoStuff lin: MsgBox "Hello"

although I don't really like line continuation, it does exit nicely and without executing everything.
but I can't add remarks within the block.

If condition1 Then _'check name and address matches
If condition2 Then _'send emails to all your friends
DoStuff lin: MsgBox "Hello"

I do try to name my functions carefully but sometimes a little extra explanation can help.

It's not the executing of the conditions that I am trying to find an alternative for but the removal of my false do_loop.

Isn't there a limit to the depth of nested loops, I know I only put 5 conditions in there but I am looking for a general case.

I did think there may be a text-book answer as quite often you see statements like 'although this works it is bad programming practice', usually with no explanation as to why. Maybe there is a book of 'good programming practice' but I have yet to find it.
0

LVL 15

Expert Comment

ID: 6298195
>       Do Until EOF(fnum)
>           Do While True
>               Line Input #1, lin
>               If Not condition1 Then Exit Do
>               If Not condition2 Then Exit Do
>               If Not condition3 Then Exit Do
>               If Not condition4 Then Exit Do
>               If Not condition5 Then Exit Do
>               DoStuff lin
>           Loop
>       Loop

>removal of my false do_loop.
Without Do Loop (note that most programmers don't like GOTOs):

Do Until EOF(fnum)
Line Input #1, lin
If Not Condition1 Then GoTo continue
If Not Condition2 Then GoTo continue
If Not Condition3 Then GoTo continue
If Not Condition4 Then GoTo continue
If Not Condition5 Then GoTo continue
DoStuff lin
continue:
Loop
0

LVL 17

Author Comment

ID: 6298515
aaaarrgghh!!!
0

LVL 17

Author Comment

ID: 6298603
re: aaarrggh

Sorry, you caught me by surprise there.

I suppose I am trying to GoTo without actually using the word.

The procedure's I write that use this type of nested statements are usually robotics that run through a sequence of actions usually performed by a user, the robotic just makes it easier and a lot faster for them.

Maybe I'm trying to hide the fact that it's sequential.
0

LVL 15

Expert Comment

ID: 6298752
>Private Sub Command2_Click()
>Dim lin As String
>Dim ra() As String
>   fnum = FreeFile
>   Open filename For Input As #fnum
>       Do Until EOF(fnum)
>           Do While True
>               Line Input #1, lin
>               If Not condition1 Then Exit Do
>               If Not condition2 Then Exit Do
>               If Not condition3 Then Exit Do
>               If Not condition4 Then Exit Do
>               If Not condition5 Then Exit Do
>               DoStuff lin
>           Loop
>       Loop
>   Close (fnum)
>End Sub

I think this is a bit better:

Private Sub Command2_Click()
Dim lin As String
Dim ra() As String
fnum = FreeFile
Open filename For Input As #fnum
Do Until EOF(fnum)
Line Input #1, lin
Do
If Not condition1 Then Exit Do
If Not condition2 Then Exit Do
If Not condition3 Then Exit Do
If Not condition4 Then Exit Do
If Not condition5 Then Exit Do
DoStuff lin
Exit Do
Loop
Loop
Close (fnum)
End Sub
0

LVL 15

Expert Comment

ID: 6298795
or something like this (uses For...Next Loop instead of Do...Loop):

Line Input #1, lin

For i = 1 To 5
Select Case i
Case 1
If Not Condition1 Then Exit For
Case 2
If Not Condition2 Then Exit For
Case 3
If Not Condition3 Then Exit For
Case 4
If Not Condition4 Then Exit For
Case 5
If Not Condition5 Then Exit For
DoStuff lin
End Select
Next
0

LVL 17

Author Comment

ID: 6299089
ameba,
re: >I think this is a bit better:

My mistake, I actually do a function call for the outer loop along the lines of:

myObject deals with opening/closing the file, what type of file it is, rem lines etc.
(hiding the nasty stuff)

I just threw the example together without noticing where line input should have gone.

well spotted.

I'm going to have to award some points for this soon and so far I've seen two offers that interest me.

KDivad's >I'm not really suggesting this, but
(hide all the nasty bit's)

but why are you not suggesting it ?

and

ameba's >Although this piece of code satisfies all the conditions, you should not use it:

if _
if _
if _

why shouldn't I use it?

0

LVL 5

Expert Comment

ID: 6299122
<<but why are you not suggesting it ?>>

Because your question seemed to indicate that you wanted to remove that bit of code, not just hide it. And now that you say MyObject hides all the nasty reading stuff, I would have thought you'd have already thought of this.
0

LVL 17

Author Comment

ID: 6299237
Where there is no 'right' answer and I just decide which answer I like best, a suggestion that goes along the lines of the way I tend to code looks like a good one.

MyObject hides some nasty stuff, but you want to see the multitude of sins that lie beneath conditions 1 to n.

Thanks for the reply I just wondered if this was a very bad way of doing things - at least I'm not the only one :7).

0

LVL 22

Expert Comment

ID: 6299252
One thing that should be considered in any "tricky" coding is future readability and maintainability.

Although the single-line multi-IF split at line-breaks looks good, it would initially confuse most programmers.  Plus, since you can't add comments, that further enhinders the ability to clear up what you're doing.

The nested IF blocks get complicated, but are at least commonly accepted as an extension of what we already do with simple nested IF blocks.

I re-iterate that if the IF block is that complicated, it may be worthwhile breaking off a chunk and dropping it into a subroutine or maybe even a function.

If condition1 _
And condition2 _
And condition3 _
And condition4 _
And condition5 Then
DoThis
End If

I suppose it's essentially the same as using the If-Then-If-Then except that it includes the endif that gives me a good feeling (confirmation) that I've actually come to the end of this mess.
0

LVL 15

Expert Comment

ID: 6299328
RobinD,
>why shouldn't I use it?

rspahitz wrote >Although the single-line multi-IF split at line-breaks looks good, it would initially confuse most programmers.

Yes, this is the reason.  Most projects are reviewed or programmed by more programmers.
And reading is done many, many times (10x) more then writing.  It is good idea to invest in readability.
0

LVL 17

Author Comment

ID: 6299362
Thanks ameba,

I quite like the look of your:

For i = 1 To 5
Select Case i
...

although I think it would be a bit of a fright to step through, I'm sure it will come in useful one day. I've never thought of directly generating the case condition like this.

I've decided to award the points to KDivad as it was an early answer and neatly replaces the false loop with a function call.

ameba - thanks for all your suggestions, look out for a 'points for' as soon as I get one in there.
0

LVL 5

Expert Comment

ID: 6299599
<surprised>

I also like the look of ameba's For...Next idea. Not sure I'd ever use it, but it does look ingenius.
0

LVL 6

Expert Comment

ID: 6301195
I have the best solution:

Call DoJob

:-)
0

LVL 5

Expert Comment

ID: 6303225
Short. To the point.

<grin>
0

LVL 22

Expert Comment

ID: 6303376
Yup, but ultimately it comes back to the accepted solution: hide it in a corner somewhere.
0

LVL 5

Expert Comment

ID: 6303403
Sure does. Just depends on how much of the code Robin is willing to (and can) hide.
0

LVL 17

Author Comment

ID: 6307445
The primary purpose of this was to create some readable, easy to step-through code in a 'good coding-practice' way.
The nearest I had got to this was by using my fake do-loop.
It works quite well, you can step through the different procedures and each one does it's own little bit in it's own little way.
The only sore thumb was the do-loop.

KDivad's suggestion placed the contents of the loop in a function - same thing happens, except that you get a perfectly valid 'Exit Function' in place of exit for.

Actually I hadn't thought of this as the procedures that are called are so high level that shoving them down a step just hadn't crossed my mind.

It's not so much about hiding code (It all has to be there for something or I wouldn't have put it in.) as keeping code that does things with other code that does similar things and giving it all access to common procedures where necessary.

I just wondered if there was a 'right' way to do it. If there is this may be it.
0

## Featured Post

Have you ever wanted to restrict the users input in a textbox to numbers, and while doing that make sure that they can't 'cheat' by pasting in non-numeric text? Of course you can do that with code you write yourself but it's tedious and error-prone â€¦
Enums (shorthand for â€˜enumerationsâ€™) are not often used by programmers but they can be quite valuable when they are. Â What are they? An Enum is just a type of variable like a string or an Integer, but in this case one that you create that containsâ€¦
Get people started with the process of using Access VBA to control Outlook using automation, Microsoft Access can control other applications. An example is the ability to programmatically talk to Microsoft Outlook. Using automation, an Access applicâ€¦
Get people started with the utilization of class modules. Class modules can be a powerful tool in Microsoft Access. They allow you to create self-contained objects that encapsulate functionality. They can easily hide the complexity of a process fromâ€¦