How to reformat this code so I don't use 'Exit Try' and 'Exit For'?

I use SonarQube to help me try and become a better programmer to program along with 'best-practice' standards.

There are business rules when a user is submitting data they enter in a datagridview that I encompass in a Try Catch block. I think the comments in the code describe fairly well what should be happening.

The problem: When I submit my project to SonarQube, it says "Exit Statements should not be used." I do not know what to use in lieu of the Exit statements, when they do exactly what I need. Can anyone point me in the right direction?

A visual representation of the Data Grid View:

Must be between 0-85             Must total to 100
+------------------+-------------+-----------------+--------------+
| Bonus Percentage | Fund Option | Fund Percentage | Election Date|
+------------------+-------------+-----------------+--------------+
|       85         |     2       |     50          |    3/20/2014 |
+------------------+-------------+-----------------+--------------+
|       85         |     5       |     50          |    3/20/2014 |
+------------------+-------------+-----------------+--------------+


Code:

tot = CInt(BonusDGV.Rows(0).Cells(0).Value)

Try
'Ensure, for all rows, Fund amount totals 100 and Bonus Percentage is between 0-85%.
For i As Integer = 0 To BonusDGV.Rows.Count - 1
'don't count rows that contain no data
If BonusDGV.Rows(i).Cells(0).Value = Nothing Then
Exit For
End If
fundTot = fundTot + CInt(BonusDGV.Rows(i).Cells(2).Value)
If tot <> CInt(BonusDGV.Rows(i).Cells(0).Value) Then
MessageBox.Show("Percentage Must Be Between 0-85%.", "Invalid Entry", MessageBoxButtons.OK, MessageBoxIcon.Error)
'Stop looping, there has been a violation in user entry (Bonus Percentage)
Exit Try
End If
Next
'Bonus Percentage must be between 0-85% // \\ Double check
If tot > 85 Or tot < 0 Then
MessageBox.Show("Percentage Must Be Between 0-85%.", "Invalid Entry", MessageBoxButtons.OK, MessageBoxIcon.Error)
Exit Try
'Total of fun amounts must = 100
ElseIf fundTot <> 100 Then
MessageBox.Show("Fund Total Must Add to 100%.", "Invalid Entry", MessageBoxButtons.OK, MessageBoxIcon.Error)
Exit Try
'Met all the business requirements, proceed to insert record
Else
For i As Integer = 0 To BonusDGV.Rows.Count - 1
'Make sure there is actually data in the row
If BonusDGV.Rows(i).Cells(0).Value = Nothing Then
Exit For
End If
cmd.Parameters("@BonusPct").Value = BonusDGV.Rows(i).Cells(0).Value
cmd.Parameters("@FundID").Value = CInt(BonusDGV.Rows(i).Cells(1).Value)
cmd.Parameters("@DefPct").Value = BonusDGV.Rows(i).Cells(2).Value
cmd.Parameters("@ElectDT").Value = BonusDGV.Rows(i).Cells(3).Value
'nonquery because we don't expect a return value
cmd.ExecuteNonQuery()
Next
End If
Catch ex As Exception
MessageBox.Show("Invalid Entry. Make sure Bonus Percentage is between 0-85% and Fund Percentage total = 100.", "Invalid Entry", MessageBoxButtons.OK, MessageBoxIcon.Error)
End Try
'dispose of the connection
conn.Close()


All of your Exit Try statements follow an error condition. So why not throw an exception with the message and display it in the Catch block instead? That's the purpose of the Try..Catch.

Also, to get rid of the Exit For, just reverse the conditional. As it is, those two statements will end the loop completely on the first empty row, not just skip it.

(Ideally, you'd want to create a custom Exception class and throw it instead of a generic Exception, but this is just to give you an idea.)

So you end up with something like this

tot = CInt(BonusDGV.Rows(0).Cells(0).Value)

Try
'Ensure, for all rows, Fund amount totals 100 and Bonus Percentage is between 0-85%.
For i As Integer = 0 To BonusDGV.Rows.Count - 1
'don't count rows that contain no data
If BonusDGV.Rows(i).Cells(0).Value IsNot Nothing Then
fundTot = fundTot + CInt(BonusDGV.Rows(i).Cells(2).Value)
If tot <> CInt(BonusDGV.Rows(i).Cells(0).Value) Then
Throw New Exception("Percentage Must Be Between 0-85%.")
End If
End If
Next
'Bonus Percentage must be between 0-85% // \\ Double check
If tot > 85 Or tot < 0 Then
Throw New Exception("Percentage Must Be Between 0-85%.")
ElseIf fundTot <> 100 Then
Throw New Exception("Fund Total Must Add to 100%.")
'Met all the business requirements, proceed to insert record
Else
For i As Integer = 0 To BonusDGV.Rows.Count - 1
'Make sure there is actually data in the row
If BonusDGV.Rows(i).Cells(0).Value IsNot Nothing Then
cmd.Parameters("@BonusPct").Value = BonusDGV.Rows(i).Cells(0).Value
cmd.Parameters("@FundID").Value = CInt(BonusDGV.Rows(i).Cells(1).Value)
cmd.Parameters("@DefPct").Value = BonusDGV.Rows(i).Cells(2).Value
cmd.Parameters("@ElectDT").Value = BonusDGV.Rows(i).Cells(3).Value
'nonquery because we don't expect a return value
cmd.ExecuteNonQuery()
End If
Next
End If
Catch ex As Exception
MessageBox.Show(ex.Message, "Invalid Entry", MessageBoxButtons.OK, MessageBoxIcon.Error)
End Try
'dispose of the connection
conn.Close()

• I thought about doing the IsNot Nothing, which I should have known better. I'll have to do more research on Throw New Exception, I'm not too familiar with that. I'm assuming it skips the rest of the code once entered and goes immediately to the Catch block. Thanks! I Appreciate it. I would love to hear more about the Exception class I should be using in lieu of these Exceptions. – Mark Mar 20 '14 at 18:31
• @MarkLaREZZA Throw is a way of manually creating an exception. You're correct that it will transfer control to the Catch block, just as if any other error in the code threw an exception. What I was referring to by custom exception is making your own class that inherits from Exception and using it. Then you have the ability to respond differently if you catch an exception of the custom class versus an unexpected exception generated elsewhere in the code. – Roger Mar 20 '14 at 18:59
• Got it. I looked at the MSDN documentation, but it's somewhat over my head and kind of lacking in my opinion. I'll have to search for a better resource. Thanks again! – Mark Mar 20 '14 at 19:12

I would seperate the error checking from the database connection.

Enum ErrorType
Success
NotBetween
End Enum

Function IsThereAnError() As ErrorType

tot = CInt(BonusDGV.Rows(0).Cells(0).Value)

'Ensure, for all rows, Fund amount totals 100 and Bonus Percentage is between 0-85%.
For i As Integer = 0 To BonusDGV.Rows.Count - 1
'don't count rows that contain no data
If BonusDGV.Rows(i).Cells(0).Value = Nothing Then
Exit For
End If
fundTot = fundTot + CInt(BonusDGV.Rows(i).Cells(2).Value)
If tot <> CInt(BonusDGV.Rows(i).Cells(0).Value) Then
Return ErrorType.NotBetween
'Stop looping, there has been a violation in user entry (Bonus Percentage)
End If
Next
'Bonus Percentage must be between 0-85% // \\ Double check
If tot > 85 Or tot < 0 Then
Return ErrorType.NotBetween
'Total of fun amounts must = 100
ElseIf fundTot <> 100 Then
'Met all the business requirements, proceed to insert record
End If

Return ErrorType.Success
End Function

Public Sub Save()

' OPEN DATABASE CONNECTION

For i As Integer = 0 To BonusDGV.Rows.Count - 1
'Make sure there is actually data in the row
If BonusDGV.Rows(i).Cells(0).Value = Nothing Then
Exit For
End If
cmd.Parameters("@BonusPct").Value = BonusDGV.Rows(i).Cells(0).Value
cmd.Parameters("@FundID").Value = CInt(BonusDGV.Rows(i).Cells(1).Value)
cmd.Parameters("@DefPct").Value = BonusDGV.Rows(i).Cells(2).Value
cmd.Parameters("@ElectDT").Value = BonusDGV.Rows(i).Cells(3).Value
'nonquery because we don't expect a return value
cmd.ExecuteNonQuery()
Next

' CLOSE DATABASE CONNECTION

End Sub

Sub DoSomething()

Select Case IsThereAnError()
Case ErrorType.Success
Save()
Case ErrorType.NotBetween
MessageBox.Show("Percentage Must Be Between 0-85%.", "Invalid Entry", MessageBoxButtons.OK, MessageBoxIcon.Error)