VB - Shorter code?

[mike2]

OK im sure that this is can get shorter :

If lstSeeds.ListCount < 16 Then
MsgBox "Not Enough Seeds", vbOKOnly, "Not Enough Seeds"
Else
If lstSeeds.ListCount > 16 Then
MsgBox "Too Many Seeds", vbOKOnly, "Too Many Seeds"
Else
If lstSeeds.ListCount = 16 Then
If lstLadder.ListCount > 8 Then
MsgBox "Too Many Ladder Rankings", vbOKOnly, "Too Many Ladder Rankings"
Else
If lstLadder.ListCount < 8 Then
MsgBox "Not Enough Ladder Rankings", vbOKOnly, "Not Enough Ladder Rankings"
Else
If lstLadder.ListCount = 8 Then
If lstWildcards.ListCount < 8 Then
MsgBox "Not Enough Wildcards", vbOKOnly, "Not Enough Wildcards"
Else
If lstWildcards.ListCount > 8 Then
MsgBox "Too Many Wildcards", vbOKOnly, "Too Many Wildcards"
Else
If lstWildcards.ListCount = 8 Then
frmBracket.Show
End If
End If
End If
End If
End If
End If
End If
End If
End If


Any ideas?

[CornedBee]

First, I think VB has an Elsif or Elseif keyword that you can use to get rid of the useless indentation and a lot of End Ifs.

Then, you have a few redundant ifs. If you first test x < n, then x > n, then you don't need to test x == n in the final case, as it's guaranteed to be true. This applies to these three lines:
If lstSeeds.ListCount = 16 Then
If lstLadder.ListCount = 8 Then
If lstWildcards.ListCount = 8 Then

[si_the_geek]

basically the main improvement can be made by using Select Case statements - which is basically a block of "If"s that check the same variable.

Also, you were doing checks that werent needed (like "If lstSeeds.ListCount = 16 Then"), as they must be true for them to be run in the first place (as you have already eliminated <16 and >16).


Select Case lstSeeds.ListCount
Case < 16: MsgBox "Not Enough Seeds", vbOKOnly, "Not Enough Seeds"
Case > 16: MsgBox "Too Many Seeds", vbOKOnly, "Too Many Seeds"
Case Else '(by now you already know it must be 16!)
Select Case lstLadder.ListCount
Case > 8: MsgBox "Too Many Ladder Rankings", vbOKOnly, "Too Many Ladder Rankings"
Case < 8: MsgBox "Not Enough Ladder Rankings", vbOKOnly, "Not Enough Ladder Rankings"
Case Else
Select Case lstWildcards.ListCount
Case < 8: MsgBox "Not Enough Wildcards", vbOKOnly, "Not Enough Wildcards"
Case > 8: MsgBox "Too Many Wildcards", vbOKOnly, "Too Many Wildcards"
Case Else
frmBracket.Show
End Select
End Select
End Select

[si_the_geek]

Originally posted by CornedBee
First, I think VB has an Elsif or Elseif keyword that you can use to get rid of the useless indentation and a lot of End Ifs.

True, you could have this:

If lstSeeds.ListCount < 16 Then
MsgBox "Not Enough Seeds", vbOKOnly, "Not Enough Seeds"
ElseIf lstSeeds.ListCount > 16 Then
MsgBox "Too Many Seeds", vbOKOnly, "Too Many Seeds"
Else
If lstLadder.ListCount > 8 Then
MsgBox "Too Many Ladder Rankings", vbOKOnly, "Too Many Ladder Rankings"
ElseIf lstLadder.ListCount < 8 Then
MsgBox "Not Enough Ladder Rankings", vbOKOnly, "Not Enough Ladder Rankings"
Else
....

I think that the Select Case version is much more readable though.

[CornedBee]

Not for me, I'm not used to expressions being allowed in a multiple-branch structure.

[abcdefg]

You should use select case because select case is fast and gains speed over nested ifs the more ifs you use. A select case is implemented as a table of pointers to all the label names you use so its real fast.

[CornedBee]

It's not implemented as such if the compiler can't translate it to that. Which is the case here, I'm pretty sure.

[dis1411]

how bout this

If lstSeeds.ListCount <> 16 Then
MsgBox "please enter 16 seeds"
ElseIf lstLadder.ListCount <> 8 Then
MsgBox "please enter 8 rankings"
ElseIf lstWildcards.ListCount <> 8 Then
MsgBox "please enter 8 wildcards"
Else
frmBracket.Show
End If

in the original code you tell the user they've entered too much or too little, but make them guess the actual number youre looking for :D