Paul Clapham wrote:You're right, it's a breeding ground for bugs. Not to mention that if you've got an employee named O'Brien then it's going to break. So clean it up by using a PreparedStatement with parameters, rather than trying to build an SQL string and get all the quotes right. Here's what it would look like:
Martin Vajsar wrote:You get the exception because you have 11 parameters (count the question marks in your statement), but only provide values for 10 of them. You need to add the 11th parameter with the original value of the Emp ID of the record you're updating (the one in the WHERE clause).
Paul Clapham wrote:You had a correct PreparedStatement before, but then you changed it to use string concatenation. Why did you do that?
And also, why did you add in the comma before the WHERE clause which makes it not valid SQL any more?
You get the exception because you have 11 parameters (count the question marks in your statement), but only provide values for 10 of them. You need to add the 11th parameter with the original value of the Emp ID of the record you're updating (the one in the WHERE clause).
Nick de Waal wrote:I am not sure what to set the 11th parameter to because it will be different each time would I just set it as the first?
Martin Vajsar wrote:
Nick de Waal wrote:I am not sure what to set the 11th parameter to because it will be different each time would I just set it as the first?
Since you already wrote the original, non-parametrized statement, you simply replace literals (which came from your text boxes) with question marks and use the setXXX methods to provide values for individual parameters (using the same values from your text boxes). This is a purely mechanical exercise, you only have to be careful and use the correct setXXX method (the correct type) - if you, for example, use setString() to define a value for numeric field, it will cause an implicit conversion in your database to happen, which is error prone and in some databases might expose you to some of the more exotic forms of SQL injection attack.
However, there is one peculiarity in your original statement, which is probably the source of confusion here. Look again at this code of yours, especially at the [Emp ID] field:
You're setting Emp ID to the value of the txtEmpNo textbox in all records that happen to have the Emp ID field equal to the txtEmpNo textbox. Therefore, nothing happens to the value of Emp ID, though, depending on the database, this could have some effects (eg. some databases might fire a trigger for this kind of update, badly coded triggers might cause further problems with this). It is therefore best to avoid these "empty updates", as it also can cause confusion about the intents of the original author of the code, since there are at least two possible interpretations of this situation:
1) The user should be able to change the value of the Emp ID field. In this case, you need to save the original value of the Emp ID field in a variable and use it in the WHERE clause of your statement, the value from the textbox will be used only to set the new value for the field.
2) The user is not supposed to change the Emp ID field. If this case, the textbox should be disabled, to indicate to the user he cannot change its value, and the Emp ID field should be dropped from the list of updated columns in the SET clause (it will remain in the WHERE clause, of course). It would be a good idea to store the original Emp ID value in a variable as in the previous case, though.
Hope this clears it up a bit.
SCJP 1.4 - SCJP 6 - SCWCD 5 - OCEEJBD 6 - OCEJPAD 6
How To Ask Questions How To Answer Questions
He's giving us the slip! Quick! Grab this tiny ad!
Smokeless wood heat with a rocket mass heater
https://woodheat.net
|