3
\$\begingroup\$

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()
\$\endgroup\$

2 Answers 2

Reset to default
3
\$\begingroup\$

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()
\$\endgroup\$
3
  • \$\begingroup\$ 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. \$\endgroup\$
    – Mark
    Commented Mar 20, 2014 at 18:31
  • \$\begingroup\$ @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. \$\endgroup\$
    – Roger
    Commented Mar 20, 2014 at 18:59
  • \$\begingroup\$ 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! \$\endgroup\$
    – Mark
    Commented Mar 20, 2014 at 19:12
0
\$\begingroup\$

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
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.