Trying to use ExecuteScalar , and get " Specified cast is not valid " error

Question

I'm trying to get product price by using product name. Below is the function I am using.

public int GetProductPrice(string ProductName)
{
    cnn.Open();
    SqlCommand cmd = new SqlCommand("SELECT ProductPrice FROM Products WHERE ProductName ='" + ProductName + "'", cnn);
    int price = (int)cmd.ExecuteScalar();
    return price;
}

Now I keep getting this error Specified cast is not valid, and I don't know why. Can someone help me ?

Solution

Firstly, you should use parameterized SQL instead of putting the parameter directly into the SQL. Also, you should use a using statement to close the command - and connection - when you're done. Oh, and create a new SqlConnection for each operation. So something like:

public int GetProductPrice(string productName)
{
    // Quite possibly extract the connection creation into a separate method
    // to call here.
    using (var conn = new SqlConnection(...))
    {
        conn.Open();
        using (var command = new SqlCommand(
            "SELECT ProductPrice FROM Products WHERE ProductName = @ProductName",
            conn))
        {
            command.AddParameter("@ProductName", SqlDbType.VarChar)
                   .Value = productName;
            object price = command.ExecuteScalar();
            // And you'd do the casting here
        }
    }
}

Next, we don't know the type of the ProductPrice field. It could be that you're getting a long returned, or perhaps it's decimal. The simplest way to find out is just to use:

object tmp = cmd.ExecuteScalar();

... and then look in the debugger. Also look at the type of the field in the database - that should really tell you what to expect. Have a look at the SqlDbType enumeration for mappings between the two.

Comments

[ShmuelCohen]

well i cant belive it , the field ProdcutPrice was Nvarchar , change it to int and everything works great, thanks

[Jon Skeet]

@user2120874: But please take the rest of the answer into consideration too - otherwise you're open to SQL injection attacks and all kinds of problems.

[ShmuelCohen]

well i will take it for my other projects , but this is a school project so they dont try to SQLI me . Thanks for the help

[System Down]

@user2120874 - "this is a school project so they dont try to SQLI me" You'd be surprised. I would. :)

[SecurityMatt]

@user2120874: You might get better marks if you show your teacher that you preventing hackers taking over the database in your project. If I were a teacher, I'd mark you down for having SQLi in your code, because when you graduate, SQLi is a HUGE business risk for whomever you end up working for.

[Jon Skeet]

@НадеждаТарашкевич: It absolutely does - but it's not a method call as per your comment. It's a property assignment, assigning the Value property in the just-added parameter.