Question Cleaning up code

[Arg81]

Below is some code I use to loop through rows in a datagridview and set the row colour subject to a "STATUS" field and also changing font colour and boldness dependant on whether a flag has been set to true.

Is this code optimal / can it be cleaned up somewhat?

Public Sub ColourMyGrid()
        Dim varStatus, varBagTemp As Integer
        Dim myFont As New Font(grdResults.Font, FontStyle.Bold)

        For Each row As DataGridViewRow In Me.grdResults.Rows
            varStatus = row.Cells.Item("Status").Value
            varBagTemp = row.Cells.Item("BagNo").Value
            If varStatus = "1" Then
                row.DefaultCellStyle.BackColor = Color.LightGreen
                varGoodBagNo = varBagTemp
            ElseIf varStatus = "2" Then
                row.DefaultCellStyle.BackColor = Color.PaleGoldenrod
                varGoodBagNo = varBagTemp
                If Convert.ToBoolean(row.Cells.Item("OOS_mo").Value) = True Then
                    row.Cells.Item("RangeMoisture").Style.Font = myFont
                    row.Cells.Item("RangeMoisture").Style.ForeColor = Color.White
                End If
                If Convert.ToBoolean(row.Cells.Item("OOS_90").Value) = True Then
                    row.Cells.Item("Range90").Style.Font = myFont
                    row.Cells.Item("Range90").Style.ForeColor = Color.White
                End If
                If Convert.ToBoolean(row.Cells.Item("OOS_56").Value) = True Then
                    row.Cells.Item("Range56").Style.Font = myFont
                    row.Cells.Item("Range56").Style.ForeColor = Color.White
                End If
                If Convert.ToBoolean(row.Cells.Item("OOS_335").Value) = True Then
                    row.Cells.Item("Range335").Style.Font = myFont
                    row.Cells.Item("Range335").Style.ForeColor = Color.White
                End If
                If Convert.ToBoolean(row.Cells.Item("OOS_17").Value) = True Then
                    row.Cells.Item("Range17").Style.Font = myFont
                    row.Cells.Item("Range17").Style.ForeColor = Color.White
                End If
                If Convert.ToBoolean(row.Cells.Item("OOS_pan").Value) = True Then
                    row.Cells.Item("RangePan").Style.Font = myFont
                    row.Cells.Item("Rangepan").Style.ForeColor = Color.White
                End If
            ElseIf varStatus = "3" Then
                row.DefaultCellStyle.BackColor = Color.IndianRed
                If Convert.ToBoolean(row.Cells.Item("OOS_mo").Value) = True Then
                    row.Cells.Item("RangeMoisture").Style.Font = myFont
                    row.Cells.Item("RangeMoisture").Style.ForeColor = Color.White
                End If
                If Convert.ToBoolean(row.Cells.Item("OOS_90").Value) = True Then
                    row.Cells.Item("Range90").Style.Font = myFont
                    row.Cells.Item("Range90").Style.ForeColor = Color.White
                End If
                If Convert.ToBoolean(row.Cells.Item("OOS_56").Value) = True Then
                    row.Cells.Item("Range56").Style.Font = myFont
                    row.Cells.Item("Range56").Style.ForeColor = Color.White
                End If
                If Convert.ToBoolean(row.Cells.Item("OOS_335").Value) = True Then
                    row.Cells.Item("Range335").Style.Font = myFont
                    row.Cells.Item("Range335").Style.ForeColor = Color.White
                End If
                If Convert.ToBoolean(row.Cells.Item("OOS_17").Value) = True Then
                    row.Cells.Item("Range17").Style.Font = myFont
                    row.Cells.Item("Range17").Style.ForeColor = Color.White
                End If
                If Convert.ToBoolean(row.Cells.Item("OOS_pan").Value) = True Then
                    row.Cells.Item("RangePan").Style.Font = myFont
                    row.Cells.Item("Rangepan").Style.ForeColor = Color.White
                End If
            End If
        Next
    End Sub

 

[JuggaloBrotha]

Well I see that the Font and ForeColor are all set to the same thing, the only thing that changes is the range, so you could split that off to a function then just call that function to set those values.

 

[Christopherx]

Okay, so you need something to generate the numbers. There is a way to shorten the number side of things.
dim x as integer

Function  Y(Byref X as integer) 
 Y = -151.7 * x ^ 3 + 1066 * x ^ 2 - 2172 * x + 1347
end function

Now, this will return the correct values,for each of the number related functions, eg:

If Convert.ToBoolean(row.Cells.Item("OOS_90").Value) = True Then
                    row.Cells.Item("Range90").Style.Font = myFont
                    row.Cells.Item("Range90").Style.ForeColor = Color.White
                End If)

So basically, for x = 1 Y will return the value of 90. So you would then have

If Convert.ToBoolean(row.Cells.Item("OOS_" & Y(x)).Value) = True Then
                    row.Cells.Item("Range" & Y(x)).Style.Font = myFont
                    row.Cells.Item("Range" & Y(x)).Style.ForeColor = Color.White
                End If

The only issue is, that every model has a percentage error, and this one has such a one that can't be rounded out by saving the resulting number as an integer first, so if someone could expand on this, and make it a little more accurate, that would be very much appreciated!

 

[JohnH]

JB is right, when refactoring look for repeated patterns. All your 'If CBool' blocks are equal except from the two string arguments, so create a StyleCell sub. It may look like this:

Sub StyleCell(ByVal row As DataGridViewRow, ByVal myFont As Font, ByVal source As String, ByVal target As String)
    If CBool(row.Cells(source).Value) Then
        row.Cells(target).Style.Font = myFont
        row.Cells(target).Style.ForeColor = Color.White
    End If
End Sub

Also, you do the same with these for status 2 and 3, usually that means do a combined expression (a), or branching out a method (b).

• ColorMyGrid may then look like this:
  
  Public Sub ColourMyGrid()
      For Each row As DataGridViewRow In Me.grdResults.Rows
          Dim status As Integer = CInt(row.Cells.Item("Status").Value)
          Dim bagTemp As Integer = CInt(row.Cells.Item("BagNo").Value)
          Select Case status
              Case 1
                  row.DefaultCellStyle.BackColor = Color.LightGreen
                  goodBagNo = bagTemp
              Case 2
                  row.DefaultCellStyle.BackColor = Color.PaleGoldenrod
                  goodBagNo = bagTemp
              Case 3
                  row.DefaultCellStyle.BackColor = Color.IndianRed
          End Select
  
          If status = 2 OrElse status = 3 Then
              Dim myFont As New Font(grdResults.Font, FontStyle.Bold)
              StyleCell(row, myFont, "OOS_mo", "RangeMoisture")
              StyleCell(row, myFont, "OOS_90", "Range90")
              'StyleCell...
              'StyleCell...
              'StyleCell...
              'StyleCell...
          End If
      Next
  End Sub
• or this:
  
  Sub StyleCells(ByVal row As DataGridViewRow)
      Dim myFont As New Font(grdResults.Font, FontStyle.Bold)
      StyleCell(row, myFont, "OOS_mo", "RangeMoisture")
      StyleCell(row, myFont, "OOS_90", "Range90")
      'StyleCell...
      'StyleCell...
      'StyleCell...
      'StyleCell...
  End Sub
  
  Public Sub ColourMyGrid()
      For Each row As DataGridViewRow In Me.grdResults.Rows
          Dim status As Integer = CInt(row.Cells.Item("Status").Value)
          Dim bagTemp As Integer = CInt(row.Cells.Item("BagNo").Value)
          Select Case status
              Case 1
                  row.DefaultCellStyle.BackColor = Color.LightGreen
                  goodBagNo = bagTemp
              Case 2
                  row.DefaultCellStyle.BackColor = Color.PaleGoldenrod
                  goodBagNo = bagTemp
                  StyleCells(row)
              Case 3
                  row.DefaultCellStyle.BackColor = Color.IndianRed
                  StyleCells(row)
          End Select
      Next
  End Sub

 

[Arg81]

sorry for late reply, thanks guys; John I used your example of (b) in the end. Actually looking at that it's easy and I've been able to achieve some extra things I didn't think I would be able to by utilising that code! cheers!