Question Validating data before executing multi looped query?

[clockwork3orange]

I wrote a VB.NET 2010 application that writes to a MS Access DB via an ODBC connection. This application is intended to take the place of the paper based Non Product PO system. The functionality in question works as follows:

A user opens the program to a blank PO form. They can search for an existing PO or create a new one. They are required to fill in the header information (stored in a table) as well the line items (saved to a different table) creating a 1 to many relationship. In the case of a new PO, after adding all the required info they would hit the insert button to save all the new records. Hitting this button will check for empty fields before creating the ODBC connection. It then tracks if you did a successful search. If so it will not try to add the header info back into the database because it already exists so it knows the users is attempting to edit or add to an existing record. If the request is new it will loop through all the line items in the PO and execute the query then it proceeds to execute the header. The issues I have seen is a user enters bad data in the header and it fails executing. So what happens is the lines items exist but no headers. When they try to fix it they get an error that the unique Id is already used and I need to go in the DB to clean it up.

Any ideas how to fix all this? Also any clean way to have one button do all updates and inserts?


If SearchCount = False Then
For i = 0 To DataGridView1.RowCount - 2
cmd.CommandText = "INSERT INTO POItems (PONumber, ItemNumber, Quantity, PartNumber, Description, UnitPrice, Total, QuantityReceived, PackingSlip, AccountCodeLine, Asset)" + _
" VALUES (" + DataGridView1.Item("PONumber", i).Value + "," + DataGridView1.Item("ItemNumber", i).Value + _
", " + DataGridView1.Item("Quantity", i).Value + ", '" + DataGridView1.Item("PartNumber", i).Value + _
"', '" + DataGridView1.Item("Description", i).Value + "', " + DataGridView1.Item("UnitPrice", i).Value + _
", " + DataGridView1.Item("Total", i).Value + ", " + DataGridView1.Item("QuantityReceived", i).Value + _
", '" + DataGridView1.Item("PackingSlip", i).Value + "', " + DataGridView1.Item("AccountCode", i).Value + _
", '" + DataGridView1.Item("Asset", i).Value + "')"
cmd.ExecuteNonQuery()
Next
cmd.CommandText = "INSERT INTO POHeader (PONumber, PODate, ShipVIA, RDDate, FOB, VQDDate, Terms, Vendor, AccountCode, " + _
"Plant, Confirmation, Certification, Requestor, Approver, Route, Notes, Void, Open, AccApp, POType )" + _
"VALUES (" + txtPONumber.Text + ", #" + DTPDate.Text + "#, '" + cbShipVIA.Text + "', #" + DTPRDD.Text + "#, '" + _
cbFOB.Text + "', #" + DTPVQDD.Text + "#, '" + txtTerms.Text + "', '" + txtVendor.Text + "', '" + txtAccount.Text + "', '" + _
cbPlant.Text + "', " + cbConfirmation.Text + ", " + cbCertification.Text + ", '" + txtRequestor.Text + "', '" + _
txtApprover.Text + "', '" + txtRoute.Text + "', '" + rtbNotes.Text + "', " + cbVoid.Text + ", " + cbOpen.Text + ", '" + txtAcctApp.Text + "', '" + cbPOType.Text + "')"
cmd.ExecuteNonQuery()

ElseIf SearchCount = True Then

For i = rows To DataGridView1.RowCount - 2
cmd.CommandText = "INSERT INTO POItems (PONumber, ItemNumber, Quantity, PartNumber, Description, UnitPrice, Total, QuantityReceived, PackingSlip, AccountCodeLine, Asset)" + _
" VALUES (" + DataGridView1.Item("PONumber", i).Value + "," + DataGridView1.Item("ItemNumber", i).Value + _
", " + DataGridView1.Item("Quantity", i).Value + ", '" + DataGridView1.Item("PartNumber", i).Value + _
"', '" + DataGridView1.Item("Description", i).Value + "', " + DataGridView1.Item("UnitPrice", i).Value + _
", " + DataGridView1.Item("Total", i).Value + ", " + DataGridView1.Item("QuantityReceived", i).Value + _
", '" + DataGridView1.Item("PackingSlip", i).Value + "', " + DataGridView1.Item("AccountCode", i).Value + _
", '" + DataGridView1.Item("Asset", i).Value + "')"
cmd.ExecuteNonQuery()
Next
End If

 

[jmcilhinney]

You should be using a transaction. The idea of a transaction is to group multiple operations together and ensure that either they all succeed or none of them do. You can begin a transaction before making any changes to the database. As you make changes, if any errors occur then you simply roll back the transaction and all your changes are undone. If you get to the end of your changes with no errors then you commit the transaction and all your changes become permanent. Using an OdbcTransaction would look something like this:

Using connection As New OdbcConnection("connection string here")
    'The connection must be open to begin a transaction.
    connection.Open()

    Dim transaction = connection.BeginTransaction()

    Try
        'Perform all your data access here.
        Using command As New OdbcCommand("SQL code here", connection)
            'Each command executed over the connection must be associated with the transaction.
            command.Transaction = transaction

            '...
        End Using

        'If we get here then no errors occurred.
        transaction.Commit()
    Catch ex As Exception
        'An error has occurred somewhere during the data access.
        transaction.Rollback()
    End Try
End Using

On an unrelated note, you could make some serious improvements to just the code you posted. NEVER use string concatenation like that to build SQL code. ALWAYS use parameters. To learn how, follow the Blog link in my signature and check out my post on Parameters In ADO.NET. In the case of the loop, you would not keep changing the CommandText. You'd set the CommandText once, add the parameters once and then set their Value in the loop.

That said, you really shouldn't be using a loop at all. Instead of calling ExecuteNonQuery in a loop, you should be using a data adapter to save the contents of a DataTable. You should create the DataTable up front and then bind it to the grid. Any changes the user makes in the grid will then be automatically pushed to the table. Once you're done, you just call Update on the data adapter and the whole lot is saved in one go.