SELECT query using SqlParameter isn't working

Question

I'm working on a select query for sql in c#. Trying to use SqlParameter, but when I debug, I throw an exception:

Must declare scalar variable "@p1".

I'm passing values in via textbox to properties in a class Customer. This is what is in my event handler:

var newCustomer = new Customer();
newCustomer.FirstName = textBoxFirstName.Text;
newCustomer.LastName = textBoxLastName.Text;
newCustomer.Address = textBoxAddress.Text;
newCustomer.City = textBoxCity.Text;
newCustomer.State = textBoxState.Text;
newCustomer.Zip = Int32.Parse(textBoxZip.Text);
newCustomer.Phone = Int64.Parse(textBoxPhone1.Text);
newCustomer.Notes = textBoxNotes.Text;

var crochetDataHandler = new CrochetDataHandler();
crochetDataHandler.InsertNewCustomer(newCustomer);

And this is the method I'm calling:

public void InsertNewCustomer(Customer customer)
{
        var insertCustomerString = 
"Insert into Customer1 (FirstName, LastName, Address, City, State, Zip, Phone, Notes) Values (@p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8)";

    SqlCommand newCust = new SqlCommand(insertCustomerString);
    newCust.Parameters.Add(new SqlParameter("@p1", customer.FirstName));
    newCust.Parameters.Add(new SqlParameter("@p2", customer.LastName));
    newCust.Parameters.Add(new SqlParameter("@p3", customer.Address));
    newCust.Parameters.Add(new SqlParameter("@p4", customer.City));
    newCust.Parameters.Add(new SqlParameter("@p5", customer.State));
    newCust.Parameters.Add(new SqlParameter("@p6", customer.Zip));
    newCust.Parameters.Add(new SqlParameter("@p7", customer.Phone));
    newCust.Parameters.Add(new SqlParameter("@p8", customer.Notes));

    newCust.CommandText.ToString();
    InsertRecord(insertCustomerString);

I have seen a few different constructors used for SqlParameter in other questions, but when I try them I get the same exception.

Answer 1

I'm not sure what your InsertRecord method is doing, but you're passing the original string insertCustomerString to it rather than your SqlCommand object. The string knows nothing about what you've done with the parameters, so if InsertRecord is expecting the string to know the parameters' values it'll be disappointed.

Answer 2

Below is how to do it.
Additionally, you shouldn't ever use raw System.Data.SqlClient objects, you should use the common interface types.

 System.Data.IDbConnection idbc = new System.Data.SqlClient.SqlConnection("connectionstring");
    System.Data.IDbCommand cmd = idbc.CreateCommand();
    cmd.CommandText = "INSERT INTO Customer1 (FirstName, LastName, Address, City, State, Zip, Phone, Notes) Values (@p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8)";
    System.Data.IDbDataParameter parm = cmd.CreateParameter();
    parm.DbType = System.Data.DbType.AnsiString;
    parm.ParameterName = "@p1";
    parm.Value = "ValueOfP1";
    cmd.Parameters.Add(parm);
    // etc.

    if(cmd.Connection.State != System.Data.ConnectionState.Open)
        cmd.Connection.Open();
    cmd.ExecuteNonQuery();

    if (cmd.Connection.State != System.Data.ConnectionState.Closed)
        cmd.Connection.Close();

Also, you should use using and lock on all objects of value ;)

    using (System.Data.IDbConnection idbc = new System.Data.SqlClient.SqlConnection("connectionstring"))
    {

        lock (idbc)
        {

            using (System.Data.IDbCommand cmd = idbc.CreateCommand())
            {
                lock (cmd)
                {

                    cmd.CommandText = "INSERT INTO Customer1 (FirstName, LastName, Address, City, State, Zip, Phone, Notes) Values (@p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8)";
                    System.Data.IDbDataParameter parm = cmd.CreateParameter();
                    parm.DbType = System.Data.DbType.AnsiString;
                    parm.ParameterName = "@p1";
                    parm.Value = "ValueOfP1";
                    cmd.Parameters.Add(parm);

                    //

                    if (cmd.Connection.State != System.Data.ConnectionState.Open)
                        cmd.Connection.Open();
                    cmd.ExecuteNonQuery();

                    if (cmd.Connection.State != System.Data.ConnectionState.Closed)
                        cmd.Connection.Close();
                }
            }
        }
    }

And as a bonus remark, you shouldn't use new System.Data.SqlClient.SqlConnection, instead you should use a factory type like:

System.Data.Common.DbProviderFactory fac = System.Data.Common.DbProviderFactories.GetFactory("System.Data.SqlClient");

            System.Data.IDbConnection idbc = fac.CreateConnection();
            idbc.ConnectionString = "ConnectionString";

Also, you can automagically map your value to the corresponding sql type:

// From Type to DBType
protected virtual System.Data.DbType GetDbType(Type type)
{
    // http://social.msdn.microsoft.com/Forums/en/winforms/thread/c6f3ab91-2198-402a-9a18-66ce442333a6
    string strTypeName = type.Name;
    System.Data.DbType DBtype = System.Data.DbType.String; // default value

    try
    {
        if (object.ReferenceEquals(type, typeof(System.DBNull)))
        {
            return DBtype;
        }

        if (object.ReferenceEquals(type, typeof(System.Byte[])))
        {
            return System.Data.DbType.Binary;
        }

        DBtype = (System.Data.DbType)Enum.Parse(typeof(System.Data.DbType), strTypeName, true);

        // Es ist keine Zuordnung von DbType UInt64 zu einem bekannten SqlDbType vorhanden.
        // http://msdn.microsoft.com/en-us/library/bbw6zyha(v=vs.71).aspx
        if (DBtype == System.Data.DbType.UInt64)
            DBtype = System.Data.DbType.Int64;
    }
    catch (Exception)
    {
        // add error handling to suit your taste
    }

    return DBtype;
} // End Function GetDbType


public virtual System.Data.IDbDataParameter AddParameter(System.Data.IDbCommand command, string strParameterName, object objValue)
{
    return AddParameter(command, strParameterName, objValue, System.Data.ParameterDirection.Input);
} // End Function AddParameter


public virtual System.Data.IDbDataParameter AddParameter(System.Data.IDbCommand command, string strParameterName, object objValue, System.Data.ParameterDirection pad)
{
    if (objValue == null)
    {
        //throw new ArgumentNullException("objValue");
        objValue = System.DBNull.Value;
    } // End if (objValue == null)

    System.Type tDataType = objValue.GetType();
    System.Data.DbType dbType = GetDbType(tDataType);

    return AddParameter(command, strParameterName, objValue, pad, dbType);
} // End Function AddParameter


public virtual System.Data.IDbDataParameter AddParameter(System.Data.IDbCommand command, string strParameterName, object objValue, System.Data.ParameterDirection pad, System.Data.DbType dbType)
{
    System.Data.IDbDataParameter parameter = command.CreateParameter();

    if (!strParameterName.StartsWith("@"))
    {
        strParameterName = "@" + strParameterName;
    } // End if (!strParameterName.StartsWith("@"))

    parameter.ParameterName = strParameterName;
    parameter.DbType = dbType;
    parameter.Direction = pad;

    // Es ist keine Zuordnung von DbType UInt64 zu einem bekannten SqlDbType vorhanden.
    // No association  DbType UInt64 to a known SqlDbType

    if (objValue == null)
        parameter.Value = System.DBNull.Value;
    else
        parameter.Value = objValue;

    command.Parameters.Add(parameter);
    return parameter;
} // End Function AddParameter


public virtual T GetParameterValue<T>(System.Data.IDbCommand idbc, string strParameterName)
{
    if (!strParameterName.StartsWith("@"))
    {
        strParameterName = "@" + strParameterName;
    }

    return InlineTypeAssignHelper<T>(((System.Data.IDbDataParameter)idbc.Parameters[strParameterName]).Value);
} // End Function GetParameterValue<T>

If you do it that way, you remain portable with all ado.net db providers, and only need to adapt the string in getfactory plus your sql code when you change the db type.

If you do it your way, you will first have to substantially change your code before you can change the sql.