HiveBrain v1.2.0
Get Started
← Back to all entries
debugMinor

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

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisandexitforhowcodeusereformatdontry

Problem

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 100 Then

Solution

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  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()

Code Snippets

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()

Context

StackExchange Code Review Q#44887, answer score: 3

Revisions (0)

No revisions yet.