All roads lead back to Rowstate
When Updates go bad
Well, not really, but if you don't get the concept of DataRowState, you'll have little fun with ADO.NET. I've written about this several times before, but it's a very persistent problem. I had my share of problems with it at first, until one day I 'got' it. I couldn't provide an accurate estimate, but I'd guess that 20-30% of the problems that I see in the newsgroups and forums are related to Rowstate issues in one way or another. Usually, it takes the form of "My update isn't working". Whenever that's a symptom, you can bet big that it's a rowstate issue (Although if the symptom is My Update is blowing up - it's ususally something related to the Insert/Update/Delete commands or connection issues). Anyway, I came across a post today where someone was having problems because all of the values that he was adding were being submitted twice to the database. Not 3 times or 4 times, exactly twice. And found it weird, I ain't hearda this symptom before. But as I read down the post, the answer became clear.
------Code giving the problem.
Sub GatherAndUpdateAnswers()
//there is some code before this that gathers all the answers and assigns
them to variables
dim newrecord as datarow = mainform.dssurveydata1.survey_data.newrow
newrecord(0) = QuestionOneAnswer
newrecord(2) = QuestionTwoAnswer
//repeats like this until twelve
mainform.dssurveydata1.survey_data.rows.add(newrecord)
end sub
Sub UpdateData()
dim pdsInsertedRows as system.data.dataset
pdsInsertedRows = mainform.dssurvey_data1.getchanges(datarowstate.added)
mainform.oledbdataadapter1.update(pdsInsertedRows)
end sub
Private Sub Form1_Closing(...)
Call UpdateData()
end sub
When the "Finish" button is clicked on the last form,
GatherAndUpdateAnswers() |
-------------------
The first problem is not an ADO.NET Issue but one of DB Design. I remember Dr. Jarman telling the class, "If you ever turn in an assignment and a table doesn't have a key, you'll fail it and won't have another opportunity to turn it in. Sure, there are times in the real world when you're going to have a table with only one row, or at most a few rows, that don't necessarily need a key. I'd argue they should but realistically, you'll run into a few times where it's not a big deal. BUT, THOSE TIMES ARE RARE INDEED. And if you get in the habit of keying your tables, the downside of adding a tiny bit of overhead in those rare situations will pale in comparison to the downside of forgetting to put a key on a table. Real programmers key their tables." Typical college professor stuff that we all heard. Well, that's the first problem although there may be some reason that the table wasn't keyed. However if it was, then an exception would be thrown when the second set of values was attemping to write back to the DB so the problem would have shown itself front and center.
The other problem is a rowstate issue. Look at the Update routine:
dim pdsInsertedRows as system.data.dataset
pdsInsertedRows = mainform.dssurvey_data1.getchanges(datarowstate.added)
mainform.oledbdataadapter1.update(pdsInsertedRows) |
What's happening here? He's calling Getchanges ie only the rows that have changed. He's using that Dataset as the parameter for the Update method. What's happening to the original rows? Nothing. When the Changes rows are updated successfully, AcceptChanges is being called on them. However the original rows still have changes. A Debug.Assert(!dssurvey_data1) will prove this. So, the user presses the Finish button and the process gets triggered. But there are still live changes in the source. Then, when the Form closes, this routine is called again. And again the updates go back to the db. Since there isn't a key on the table, nothing stops it from adding the same rows again. The easy way to handle this is to try/catch the Update, and if it's successful, call Acceptchanges on the source dataset.
Otherwise, his approach makes perfect sense. If the user makes changes but doesn't hit finish, then those changes should be submitted to the db (Actually, I don't know what the h3ll the business rule is in this case, but in many cases that's a common thing to do). And if it weren't for the Finish Button logic, then this is something that EASILY could have krept into a production app (it may well be already). Fortunately, the fact that he hit the button and then closed the form tipped him off to a problem.
I would add as a general design suggestion that Update is a risky operation. After all, your update syntax could be bad, you could pass in some values that violate keys (although you shouldn't if you keep the same constraints on the DataTable that you have on the db), the connection pool could be corrupted, server could be down - all sorts of crap. Now, his idea of getting the changes only is a good one. That way if they Update blows up in the middle of things, you don't lose all of your rowstate assuming you need it. In a Transaction scenario - this becomes increasinly important b/c those rows that were updated will be rolled back, but only on the server - so you lost your client changes and getting them back ain't gonna happen. If it's not in a transaction, you're safe b/c the changes were submitted to the db so your datatable and your db will still match.
Fortunately the difference between a potentially ugly problem and a working solution is relatively simple - at a minimum it can be handled with one line of code - ideally it would be addressed at the server as well - just in case. But seriously, this is a prime example of where fairly decent logic can get really ugly because of a tiny oversight. Imagine if the user kept hitting that button over and over. Then it wouldn't be 2 times the data - it'd be number_of_clicks +1.
Finally, I could use this as an opportunity to illustrate the value of assertions- because a Debug.Assert(DataSet.HasChanges()); before the Update (which should pass) and afterward (!DataSet.HasChanges()); which would fail, would prove to you that you were getting what you wanted. You could also Assert that the number of Records affected by the Update was > 0 (put this in an if statement though b/c if you intended to have no changes you don't want to give a false positive) if(DataSet.HasChanges()){ //Call Update, Debug.Assert(!DataSet.HasChanges());