1

I am trying to pass a decimal value from C# to an SQL database. I build up an INSERT query string and send it along using a SqlCommand. The problem I'm having is that when I enter the values in my Windows Form they are saved with a comma separating the decimal point (i.e. 55,679). This is evident in the SQL query string (I've checked with debugging). But it seems as though the database query expects a decimal point (i.e. 55.679). It's probably got to do with my region and the standard there (i.e. Europe uses "," as the decimal separator). Anyone know how to set this?

The code for the function is below. The problems exist in the Vmax, Vmin, Imax.

public bool addBattery(Battery b)
    {
        bool outcome = false;

        string sql = @"INSERT INTO [BATTERY_INFO]
            (BatteryType, VoltageMax, VoltageMin, Capacity, CurrentMax, Manufacturer, Serial)
            VALUES ('" + b.BatteryType.ToString() + "'," + b.Vmax + "," + b.Vmin + "," 
            + b.Capacity + ","+b.Imax+ ",'"+b.Manufacturer.ToString()+"','"+b.Serial.ToString()+"')";

        SqlCommand command = new SqlCommand(sql, this.sql_conn);

        try
        {
            command.ExecuteNonQuery();
            MessageBox.Show("Inserted Battery Successfully");
            outcome = true;
        }
        catch (Exception e)
        {
            MessageBox.Show("SQL INSERT error \n\n" + e.Message);
        }


        return outcome;
    }
itstudes
  • 411
  • 5
  • 14
  • 10
    Please use Parameterized Queries and not String Concat : https://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqlcommand.parameters(v=vs.110).aspx –  Jun 29 '16 at 12:33
  • 5
    As @Stanley suggested, parameterized queries will solve the problem of decimal separators and help improve database performance too. – Dan Guzman Jun 29 '16 at 12:34
  • 3
    Can you tell us how the Battery class looks like? And also, there are better ways to do the insert.. try something like this: new SqlParameter("@VmaxParameter", SqlDbType.Decimal, b.VMax), – hbulens Jun 29 '16 at 12:35
  • I will do that, thanks. Just out of interest, do parameterized queries add security to the software too? I read in [this thread](http://stackoverflow.com/questions/13732665/insert-numerical-decimal-data-from-textbox-values) that they do. Can someone send a link confirming this? – itstudes Jun 29 '16 at 12:53
  • @tdwolff: Yes, this also adds security, imagine what would happen if you get the value `666,'uh','oh'); DROP TABLE BATTERY_INFO; --` in the `b.Imax` value. – TToni Jun 29 '16 at 13:14

1 Answers1

0

As pointed out in the comments, using a parametrized query will solve the problems with decimals as well as prevent possible security risks such as sql injections. The method would look like this.

I assume you Open() and Close() the connection elsewhere as I do not see that inside your code.

        {
        bool outcome = false;
        SqlCommand cmd = new SqlCommand();
        cmd.Connection = this.sql_conn;

        cmd.Parameters.AddWithValue("BatteryType", b.BatteryType);
        cmd.Parameters.AddWithValue("VoltageMax", b.Vmax);
        cmd.Parameters.AddWithValue("Capacity", b.Capacity);
        cmd.Parameters.AddWithValue("CurrentMax", b.Imax);
        cmd.Parameters.AddWithValue("Manufacturer", b.Manufacturer.ToString());
        cmd.Parameters.AddWithValue("Serial", b.Serial.ToString());

        try
        {
            cmd.ExecuteNonQuery();
            MessageBox.Show("Inserted Battery Successfully");
            outcome = true;
        }
        catch(Exception e)
        {
            MessageBox.Show("SQL INSERT error \n\n" + e.Message);
        }

        return outcome;
    }
pijemcolu
  • 2,257
  • 22
  • 36
  • Thanks for the answer. The connection is opened at the start of the program and is maintained open whilst the application runs. Is this good practice or not? It is closed when the application is closed. The application is used to monitor live data from some hardware (which posts to the database from another location). That's why I just keep the connection open. – itstudes Jun 30 '16 at 07:08
  • @tdwolff - Not really, ideally you wrap your connection in a `using()` statement. You open late, and close as soon as possible. One could argue that you can keep stuff open as long as you know what are you doing, but that's a risky approach. For how long will you know? How maintanable are global variables such as that connection of yours? Does it have to be global? No it doesn't. Keep your variables as local as possible. Here's an [SO question and answers](http://stackoverflow.com/questions/4111594/why-always-close-database-connection) which talk about connection handling. – pijemcolu Jun 30 '16 at 07:17