Setting fields, with default values, before saving a row

Question

I would like to find out if there's a cleaner way to go about validating input before it goes into a function. What I'm doing seems like a lot of code, I'm sure there's a better way. I would also really appreciate some constructive criticism as my goal is to grow as a developer so anything you may see wrong with it or something you would've done better please mention it.

The code below does get the job done. I have tested it and it works, however I would like to improve its beauty and efficiency.

 Private Sub dgvproformas_CellContentClick(sender As Object, e As DataGridViewCellEventArgs) Handles dgvproformas.CellContentClick
 Dim colname As String = ""
 colname = dgvproformas.Columns(e.ColumnIndex).Name
 Select Case colname
 Case "btnsave"
 Dim jobid As Integer = 0
 Dim proformano As String = ""
 Dim paymentreceiveddate As DateTime = Nothing
 Dim paymentreceived As Boolean = False
 Dim proformarequired As Boolean = False
 Dim proformaraised As Boolean = False
 jobid = dgvproformas.Item("job id", e.RowIndex).Value
 If Not IsDBNull(dgvproformas.Item("proformano", e.RowIndex).Value) Then
 proformano = dgvproformas.Item("proformano", e.RowIndex).Value
 End If
 If Not IsDBNull(dgvproformas.Item("proformapaymentreceived", e.RowIndex)) Then
 paymentreceived = dgvproformas.Item("proformapaymentreceived", e.RowIndex).Value
 End If
 If Not IsDBNull(dgvproformas.Item("proformarequired", e.RowIndex).Value) Then
 proformarequired = dgvproformas.Item("proformarequired", e.RowIndex).Value
 End If
 If Not IsDBNull(dgvproformas.Item("proformaraised", e.RowIndex).Value) Then
 proformaraised = dgvproformas.Item("proformaraised", e.RowIndex).Value
 End If
 If Not IsDBNull(dgvproformas.Item("proformapaymentreceiveddate", e.RowIndex).Value) Then
 paymentreceiveddate = dgvproformas.Item("proformapaymentreceiveddate", e.RowIndex).Value
 End If
 If MsgBox("Are you sure you'd like to save this record?", vbYesNo) = vbYes Then
 updateProforma(jobid, proformano, paymentreceived, proformaraised, proformarequired, paymentreceiveddate)
 End If
 Case Else
 End Select
End Sub

Answer 1

The problem with your code is that you keep repeating the same code over and over again.

If Not IsDBNull(dgvproformas.Item("columnName", e.RowIndex).Value) Then
 columnVar = dgvproformas.Item("columnName", e.RowIndex).Value
End If

What you need is function that does the same thing, so that you have code that looks more like this:

columnVar= fn(dgvproformas, "columnName", e.RowIndex)

Except we want to keep type safety so, a bit more like:

columnVar = fn(Of Integer)(dgvproformas, "columnName", e.RowIndex)

Personally, I like extension methods for something like this, and would create an extension method for it. I would almost certainly name it GetValueOrDefault.

I believe you can eliminate the rowindex if you get the row, so combining the two...

Dim row = dgvproformas.Rows(e.RowIndex)
columnVar = row.GetValueOrDefault(Of Integer)("columnName")

Finally, I like meaningful names and dislike magic strings, so I like Nameof...

Dim row = dgvproformas.Rows(e.RowIndex)
columnName = row.GetValueOrDefault(Of Integer)(NameOf(columnName))

Answer 2

Private Sub dgvproformas_CellContentClick(sender As Object, 
 e As DataGridViewCellEventArgs) Handles dgvproformas.CellContentClick
 ' I think you don't need colname variable at all
 ' You can just check for proper column name and exit if it is not
 ' And get rid of Select Case block
 If dgvproformas.Columns(e.ColumnIndex).Name.Equals("btnSave") = False Then
 Exit Sub
 End If
 ' Now you can get instance of DataGridViewRow 
 ' With which you can get column values without everytime providing RowIndex
 Dim row As DataGridViewRow = dgvproformas.Rows(e.RowIndex)
 ' Then instead of using six parameters for updateProforma method
 ' Create a class which represent those values
 ' If you change type of the proerties to be Nullable, then
 ' you don't need to validate against null values
 Dim data As New ProformaData With
 {
 .JobId = row.Cells("job id").Value,
 .Proformano = row.Cells("proformapaymentreceiveddate").Value,
 .PaymentReceivedDate = row.Cells("proformano").Value,
 .PaymentReceived = row.Cells("proformapaymentreceived").Value,
 .ProformaRequired = row.Cells("proformarequired").Value,
 .ProformaRaised = row.Cells("proformaraised").Value
 }
 ' Then you can change update updateProforma method
 ' to accept instance of ProformaData instead of six parameters
 If MsgBox("Are you sure you'd like to save this record?", vbYesNo) = vbYes Then
 ' Values of Nullable you can get by GetValueOrDefault method
 ' which will return default value if Nullable has no value
 ' False for Boolean, 0 for Integer, null for String
 ' Only for string you will use If statement to get empty string instead of null
 Dim proformano As String = If(data.Proformano.GetValueOrDefault, String.Empty)
 updateProforma(data.JobId.GetValueOrDefault(), 
 proformano, 
 data.PaymentReceived.GetValueOrDefault(),
 data.ProformaRaised.GetValueOrDefault(),
 data.ProformaRequired.GetValueOrDefault(), 
 data.PaymentReceivedDate.GetValueOrDefault())
 End If 
End Sub
Public Class ProformaData
 Public Property JobId As Integer?
 Public Property Proformano As String
 Public Property PaymentReceivedDate As DateTime?
 Public Property PaymentReceived As Boolean?
 Public Property ProformaRequired As Boolean?
 Public Property ProformaRaised As Boolean?
End Class

Then I suggest you set "Option Strict On" in your project or in the one file at the time. "Option Strict On" will give fast feedback (during compile time) about possible type casting/converting errors.

• Heslacher: Aside from mentioning Option Strict On, which is the reason you don't get a downvote from me, you only have presented an alternative solution, but haven't reviewed the code. Please explain your reasoning (how your solution works and why it is better than the original) so that the author and other readers can learn from your thought process.