1

I have a function called "InsertEmpolyee" that receives parameters to fill a query string. The thing is, I want to make some of these variables optional, in other words, I would like to be able to call the function without passing values to those parameters and still make the correct query string by inserting NULL into the database.

This is the function InsertEmployee

public int InsertEmployee(string FirstName, char Minit, string LastName, int SSN
 , int? Salary)
{
    string query = "INSERT INTO Employee (Fname, Minit, Lname, SSN, Salary) " +    "Values ('" + FirstName + "','" + Minit + "','" + LastName + "'," + Salary + ");";
    return model.ExecuteNonQuery(query);
}

And here is how I call it.

int res = Controlobj.InsertEmployee(txtbox_FirstName.Text, txtbox_Minit.Text[0],
                   txtbox_LastName.Text, Int32.Parse(txtbox_SSN.Text), null);

I have tried to do the following

if (!Salary.HasValue)
            Salary = DBNull.Value;

But it gives me the following error "Can't implicitly convert system.DBNull to int?"

How can I fix this? And is there is a better way to do this?

TheVillageIdiot
  • 40,053
  • 20
  • 133
  • 188
Sayed Alesawy
  • 425
  • 2
  • 6
  • 18
  • 4
    Do not use string concatenation for your queries, use parameterized queries instead. This will ensure your code is not vulnerable to sql injection attacks. It will also solve your problem because you can then pass in `System.DBNull.Value` as the parameter value when one of your incoming parameters is `null`. – Igor Oct 24 '17 at 21:14
  • Is `model` an instance of a `SqlCommand` ? – Igor Oct 24 '17 at 21:16
  • @Igor No, it's a class I have written. I will read about parameterized queries. Actually I knew that string concatenation is vulnerable for SQL injections, but this is literally my first time dealing databases. – Sayed Alesawy Oct 24 '17 at 21:19
  • Cast DBNull.Value to object: (object)DBNull.Value – Sergei G Oct 24 '17 at 21:24
  • Why recreate `SqlCommand`? If you want to abstract it then use `DbCommand` or `IDbCommand` which are already abstracted types. You are stripping away core functionality that *you need*. See [Bobby Tables](https://xkcd.com/327/) – Igor Oct 24 '17 at 21:27
  • @YpsilonIV I have also tried that, it gives the same error but regarding type "object" instead of "int?", I have also tried casting to "object?" with the same results. – Sayed Alesawy Oct 24 '17 at 21:28
  • I think this https://stackoverflow.com/questions/11018939/insert-a-null-value-to-an-int-column-in-sql-server should work for you – Sergei G Oct 24 '17 at 21:30
  • Again, you *need to use parameters*. The *initial* suggested solution by @YpsilonIV is wrong and can't work in this manner. Learn how to use parameters, it is not that difficult to do. – Igor Oct 24 '17 at 21:31
  • @Igor Yes I will. Thanks. – Sayed Alesawy Oct 25 '17 at 06:34

1 Answers1

7

Your code doesn't only fail on null, but on strings containing apostrophes. There could be other pitfalls as well. That's why we use parameters.

public int InsertEmployee(string Fname, char Minit, string Lname, int SSN, int? Salary)
{
    return model.ExecuteNonQuery(
        @"
            INSERT INTO Employee (
                       Fname, Minit, Lname, SSN, Salary
                   ) VALUES (
                       @Fname, @Minit, @Lname, @SSN, @Salary
                   )
        ",
        new SqlParameter("@Fname",  SqlDbType.VarChar) { Value = (object)Fname  ?? System.DBNull.Value },
        new SqlParameter("@Minit",  SqlDbType.VarChar) { Value =         Minit                         },
        new SqlParameter("@Lname",  SqlDbType.VarChar) { Value = (object)Lname  ?? System.DBNull.Value },
        new SqlParameter("@SSN",    SqlDbType.Int    ) { Value =         SSN                           },
        new SqlParameter("@Salary", SqlDbType.Int    ) { Value = (object)Salary ?? System.DBNull.Value });
}
ikegami
  • 367,544
  • 15
  • 269
  • 518
  • This is not a constructor of `SqlParameter` that you should be using. It can cause very confusing behaviour because it infers the type from the value passed in, but the value passed in may be `null`, which doesn't have any type. (BTW, the link I included earlier was not a good one, please ignore that, now edited out.) –  Oct 24 '17 at 21:22
  • I have written the model class myself, and thus the function ExecuteNonQuery takes a string as a parameter. What would be the suitable parameter for your proposed method? – Sayed Alesawy Oct 24 '17 at 21:24
  • 1
    Then you need to fix ExecuteNonQuery. There's a reason SqlCommand.ExecuteNonQuery takes parameters. – ikegami Oct 24 '17 at 21:25
  • @hvd, Addressed. – ikegami Oct 24 '17 at 21:31
  • Looks good to me. You could alternatively have kept it inline with `new SqlParameter(...) { Value = ... }` but a separate method is of course perfectly fine as well. –  Oct 24 '17 at 21:42
  • @hvd, Awesome! :) I was not aware of that. Answer updated. – ikegami Oct 24 '17 at 21:42
  • The "Value = (object)Fname ?? System.DBNull.Value" part produces an error that says "Can't convert from System.Data.SqlClinet.SqlParameter to string". Any idea what's the problem? – Sayed Alesawy Oct 27 '17 at 11:45