modified money in CRUD is rising C#

Question

I have a Window Application in ado.net with adding, modifying and deleting rows for a few tables. My problem is that after modifying a table with money type, the money value is much bigger after the operation.

String sql = "UPDATE kierowca SET imie='" + txtImie.Text + "',nazwisko='" + txtNazwisko.Text + "',data_zatrudnienia='" + txtData.Text + "',pensja='" + Convert.ToDecimal(txtPensja.Text) + "' WHERE imie='" + listBox2.SelectedItem.ToString() + "' AND nazwisko='" + listBox3.SelectedItem.ToString() + "';";
conn.Open();
cmd = new SqlCommand(sql, conn);
cmd.ExecuteNonQuery();
cmd.Clone();
conn.Close();

pensja in my table is type of money. What am I dong wrong?

First, I would suggest using parameter binding and command objects over string concatenation. It is safer and easier to read and maintain. Second, try setting a break point and getting the final string value of your `sql` field. Check the final query to ensure it is executing what you are expecting. — gmiley, May 14 '16 at 12:34
First thing to fix: stop building SQL like that. Use parameterized SQL instead. That may or may not fix the problem, but it should *absolutely* be the first thing you change. You should also use `using` statements for your connection and command. — Jon Skeet, May 14 '16 at 12:34
Convert.ToDecimal(txtPensja.Text) can easily end in Culture problems. Think at thousender separators and some culture use "," others "." ..... This could also be the problem. — Cyril Iselin, May 14 '16 at 12:37
Also a good point @CyrilIselin especially given the query is... Polish? This ends up being a bigger problem here due to the forced conversion to string related to performing string concatenation instead of bound, typed parameters. — gmiley, May 14 '16 at 12:39

Steve · Accepted Answer · 2016-05-14T13:25:34.583

I would give you an example of a parameterized query using your data

String sql = @"UPDATE kierowca SET imie=@imie,nazwisko=@nazwisko,
               data_zatrudnienia=@data_zatrudnienia,pensja=@pensja
               WHERE imie=@search1 AND nazwisko=@search2";

using(SqlConnection con = new SqlConnection(......))
using(SqlCommand cmd = new SqlCommand(sql, con))
{
    cmd.Parameters.Add("@imie", SqlDbType.NVarChar).Value = txtImie.Text;
    cmd.Parameters.Add("@nazwisko", SqlDbType.NVarChar).Value = txtNazwisko.Text;
    cmd.Parameters.Add("@data_zatrudnienia", SqlDbType.NVarChar).Value = txtData.Text;
    cmd.Parameters.Add("@pensja", SqlDbType.Decimal).Value = Convert.ToDecimal(txtPensja.Text);
    cmd.Parameters.Add("@search1", SqlDbType.Decimal).Value = listBox2.SelectedItem.ToString();
    cmd.Parameters.Add("@search2", SqlDbType.Decimal).Value = listBox3.SelectedItem.ToString();
    con.Open();
    int rowsChanged = cmd.ExecuteNonQuery();
    MessageBox.Show("Updated " + rowsChanged + " rows");
}

Notice that I assume two things here.
The Convert.ToDecimal doesn't fails (better use decimal.TryParse to test if the input is indeed a decimal value).
The other fields involved in your query are all of type text (nvarchar on db)

Why this should work? Because with this code a parameter of type decimal and whose value is a decimal value is passed to the database engine. So the engine don't need to convert a string back to a decimal. This conversion could easily fails or give incorrect results if the a locale decimal point (comma) is not interpreted correctly by the database conversion code

Of course if your fields are of different type you should change the SqlDbType value in all the affected parameters

modified money in CRUD is rising C#

1 Answers1