Validation code storing empty data in database

Question

I wrote a simple code for validation with VB.NET. This code is storing EMPTY data in the database table. How do I avoid this, and what code do I write after the else if statements? i.e after the Phone no validation.

This is the code below:

 Protected Sub Button1_Click(sender As Object, e As System.EventArgs) Handles Button1.Click
        If Len(Trim(txtName.Text)) = 0 Then
            MsgBox("Enter Name", MsgBoxStyle.Critical, "Error")
            txtName.Focus()
        ElseIf Len(Trim(txtAge.Text)) = 0 Then
            MsgBox("Enter Age", MsgBoxStyle.Critical, "Error")
            txtAge.Focus()
        ElseIf Len(Trim(txtPhone.Text)) = 0 Then
            MsgBox("Enter Phone", MsgBoxStyle.Critical, "Error")
            txtPhone.Focus()
        Else
            Dim blnFlag As Boolean = False
            MsgBox("Enter the details", MsgBoxStyle.Critical, "Error")
        End If
        Try
            Dim strCommand As String
            strCommand = "Insert into [Validation] ([Name],[Age],[Phone]) VALUES"
            strCommand = strCommand & "('" & Trim(txtName.Text) & "','" & Trim(txtAge.Text) & "','" & Trim(txtPhone.Text) & "')"
            Dim StrConnection As String
            StrConnection = ConfigurationManager.ConnectionStrings("ConnectionString").ToString
            Dim cnValidation As New SqlClient.SqlConnection(StrConnection)
            If (cnValidation.State = ConnectionState.Closed) Then
                cnValidation.Open()
            End If
            Dim cmdEmployee As New SqlClient.SqlCommand(strCommand, cnValidation)
            cmdEmployee.ExecuteNonQuery()
            cnValidation.Close()
            MsgBox("Save Successful", MsgBoxStyle.Information, "Success")
        Catch ex As Exception
            MsgBox("Save failed " & ex.Message, MsgBoxStyle.Critical, "Failed")
        End Try

Not sure why you need the last Else and it would be a good idea to use SqlParameter with your command — Fred
– Fred, Commented Dec 18, 2013 at 9:36

Tim Schmelter · Accepted Answer · 2013-12-18 10:54:22Z

2

The easiest approach is, return from the method after your validation failed:

If Len(Trim(txtName.Text)) = 0 Then
    MsgBox("Enter Name", MsgBoxStyle.Critical, "Error")
    txtName.Focus()
    Return
ElseIf Len(Trim(txtAge.Text)) = 0 Then
    MsgBox("Enter Age", MsgBoxStyle.Critical, "Error")
    txtAge.Focus()
    Return
ElseIf Len(Trim(txtPhone.Text)) = 0 Then
    MsgBox("Enter Phone", MsgBoxStyle.Critical, "Error")
    txtPhone.Focus()
    Return
Else
    Dim blnFlag As Boolean = False
    MsgBox("Enter the details", MsgBoxStyle.Critical, "Error")
    Return
End If

However, the "Enter the details" part is not clear. Why do you show the error-MessageBox always? There seems to be a logical problem. I'll ignore this part henceforth.

Here's a more readable .NET version of your code:

Dim validName = Not String.IsNullOrWhiteSpace(txtName.Text)
Dim validAge = Not String.IsNullOrWhiteSpace(txtAge.Text)
Dim validPhone = Not String.IsNullOrWhiteSpace(txtPhone.Text)
Dim isValid = validName AndAlso validAge AndAlso validPhone

If Not isValid Then
    If Not validName Then
        MsgBox("Enter Name", MsgBoxStyle.Critical, "Error")
        txtName.Focus()
    ElseIf Not validAge Then
        MsgBox("Enter Age", MsgBoxStyle.Critical, "Error")
        txtAge.Focus()
    ElseIf Not validPhone Then
        MsgBox("Enter Phone", MsgBoxStyle.Critical, "Error")
        txtPhone.Focus()
    End If
    Return ' return from this method '
Else
    ' insert into DB '
    ' .... '
End If

Side-note: you should really use sql-parameters even if this a windows-application. It will not only prevent you from sql-injection attacks but can also prevent localization issues (f.e. with datetime):

Try
    Dim newIdenity As Int32 ' determine new ID generated from database '
    Dim strCommand = "Insert into [Validation] ([Name],[Age],[Phone]) VALUES (@Name,@Age,@Phone)"
    Dim StrConnection As String = ConfigurationManager.ConnectionStrings("ConnectionString").ToString
    Using cnValidation = New SqlClient.SqlConnection(StrConnection)
        Using cmdEmployee = New SqlCommand(strCommand, cnValidation)
            cnValidation.Open()
            cmdEmployee.Parameters.AddWithValue("@Name",txtName.Text)
            cmdEmployee.Parameters.AddWithValue("@Age",Int32.Parse(txtAge.Text))
            cmdEmployee.Parameters.AddWithValue("@Phone",txtPhone.Text)
            newIdenity = DirectCast(cmdEmployee.ExecuteScalar(), Int32)
        End Using
    End Using
    MsgBox("Save Successful", MsgBoxStyle.Information, "Success")
Catch ex As Exception
    MsgBox("Save failed " & ex.Message, MsgBoxStyle.Critical, "Failed")
End Try

You should use the Using-statement for everything implementiong IDisposable which disposes unmanaged resources, it'll also close the connection, even on error.

I've also shown how you can determine the newly created identity value from an IDENTITY column in sql-server.

Note that i have parsed txtAge.Text to Int32 since i assume that the type of the columns in the database is actually int. If this is not the case remove the Int32.Parse. In general you should always provide the correct type as parameter.

edited Dec 18, 2013 at 10:54

answered Dec 18, 2013 at 9:31

Tim Schmelter

462k79 gold badges719 silver badges980 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Adarsh Shah Over a year ago

Yes you need to return from the method whenever your code goes into any of the If or ElseIf.

Tim Schmelter Over a year ago

@AdarshShah: or use my second approach ;)

Adarsh Shah Over a year ago

Yes but the only problem I have with that is if someone adds code after the else block in future it will still be called if the validation fails. I think the 1st approach is more maintainable.

Tim Schmelter Over a year ago

@AdarshShah: Point taken, edited my answer to add the Return statement in the Not isValid-block. But note that this could also be incorrect if OP wants to execute code after the Else block even if validation failed. The only part that should not execute is the insert which is ensured by the If/Else.

Collectives™ on Stack Overflow

Validation code storing empty data in database

1 Answer 1

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related