0

I have code that checks for unique values when the user updates an ID field, but I am so new I am wondering if there is a better way.

private void tbPrinterID_Validating(object sender, CancelEventArgs e)
{
    using (SqlConnection conn = new SqlConnection(Properties.Settings.Default.LazerMaintenance_Conn))
    {
        try
        {
            string query = "SELECT COUNT(*) as Count FROM Printers WHERE PrinterID = '" + tbPrinterID.Text + "'";
            SqlDataAdapter da = new SqlDataAdapter(query, conn);
            DataTable dt = new DataTable();
            da.Fill(dt);
            if ((Int32)dt.Rows[0]["Count"] > 0)
            {
                MessageBox.Show("There is already a printer with ID = " + tbPrinterID.Text);
            }
        }
        catch (Exception ex)
        {
            MessageBox.Show("Error occured! : " + ex);
        }
    }
}
Lauren Rutledge
  • 1,195
  • 5
  • 18
  • 27
  • 4
    Using a DataTable is very wasteful. A SqlCommand with ExecuteScalar is much better. And do use parameters with it, think about what happens if someone writes `‘; drop table printers;--` as the text... – Sami Kuhmonen Nov 27 '18 at 17:29

1 Answers1

1

Your example is vulnerable to SQL injection, I suggest reading this What are good ways to prevent SQL injection?.

You can make the query a bit more idiomatic:

var sql = "SELECT 1 FROM Printers WHERE PrinterID = @IdToCheck";

using (var command = new SqlCommand(sql, con))
{
    command.Parameters.AddWithValue("@IdToCheck", tbPrinterID.Text);
    con.Open();
    using (var reader = command.ExecuteReader())
    {
        while (reader.Read())
        {
         ..........
        }
    }
}
Alexander Pope
  • 1,134
  • 1
  • 12
  • 22
  • Not related, but using `AddWithValue` isn't best choice if you want improve performance. – Fabio Nov 28 '18 at 08:51
  • @Fabio I like AddWithValue for simplicity, for those interested, here is a discussion on AddWithValue vs Add https://stackoverflow.com/questions/21110001/sqlcommand-parameters-add-vs-addwithvalue – Alexander Pope Nov 28 '18 at 09:02
  • `AddWithValue` will do whole work for you and as usually will do it slightly wrong, which can affect sql queries performance. Instead you should create SqlParameter and then add it to the parameters collection. By creating it "manually" you will be on full control of what you do and how. – Fabio Nov 28 '18 at 09:11