# Setting fields, with default values, before saving a row

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

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

End If

If MsgBox("Are you sure you'd like to save this record?", vbYesNo) = vbYes Then
End If

Case Else

End Select

End Sub


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

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,
.ProformaRequired = row.Cells("proformarequired").Value,
.ProformaRaised = row.Cells("proformaraised").Value
}

' Then you can change update updateProforma method

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.ProformaRaised.GetValueOrDefault(),
data.ProformaRequired.GetValueOrDefault(),
End If
End Sub

Public Property JobId As Integer?
Public Property Proformano  As String
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.

• 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. – Heslacher Dec 28 '17 at 6:36