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

Question

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)

'Business Rules Try/Catch block
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()

Answer 1

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)

'Business Rules Try/Catch block
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()

Answer 2

I would seperate the error checking from the database connection.

Enum ErrorType
    Success
    NotBetween
    NotAdding
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
        Return ErrorType.NotAdding
        '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)
        Case ErrorType.NotAdding
            MessageBox.Show("Fund Total Must Add to 100%.", "Invalid Entry", MessageBoxButtons.OK, MessageBoxIcon.Error)
    End Select