Hi guys just have a question regarding parameterized queries.

[sock1992]

Initially I was concatenating my SQL queries, i was then informed that it would be best to use parameterized queries to prevent SQL Injection. I've now done that and everything works fine, however when posting on a forum yesterday about one of my queries not working properly, a person told me that i should be using SSMS to write my SQL queries and then paste them back into a string variable in visual studio?

I'm gonna just paste some my code below, can someone tell me if I've done everything correctly? I've written all my queries in code, and it would suck if i had to go back and change everything, seeing as my deadline is next week Thanks guys!

public partial class adminLogin : Form
    {
        SqlCommand cmd;
        private static IsqlDataFunctions _isqlDataFunctions;
        public adminLogin(IsqlDataFunctions dataFunctions)
        {
            _isqlDataFunctions = dataFunctions;
        }
        public static void creationOfSqlDataFunctions()
        {
            SqlDataFunctions sqlDataFunctions = new SqlDataFunctions();
            new adminLogin(sqlDataFunctions);
        }

        public adminLogin()
        {
            creationOfSqlDataFunctions();
            InitializeComponent();
        }

        private void adminSignUpBtn_Click(object sender, EventArgs e)
        {
            if (txtAdminFirstName.Text == "" || txtAdminLastName.Text == "" || comboBoxAdminPosition.Text == "" || txtAdminPass.Text == "" || txtAdminContact.Text == "" || txtAdminEmail.Text == "")
            {
                MessageBox.Show("Error: please ensure all fields have been entered!");
            }
            else
            {          
                try
                {
                    string query_1 = "Insert into employee  (firstName, lastName, position, contactNumber) VALUES (@firstName, @lastName, @position, @contact)";
                    string query_2 = "select employeeId FROM employee WHERE employeeId = (SELECT MAX(employeeId) FROM employee)";
                    string query_3 = "Insert into employeeLogin (email, password, employeeId) VALUES (@email, @password, @employeeId)";

                    _isqlDataFunctions.GetConnection().Open();
                    cmd = new SqlCommand(query_1, _isqlDataFunctions.GetConnection());
                    cmd.Parameters.AddWithValue("@firstName", txtAdminFirstName.Text.Trim());
                    cmd.Parameters.AddWithValue("@lastname", txtAdminLastName.Text);
                    cmd.Parameters.AddWithValue("@position", comboBoxAdminPosition.Text);
                    cmd.Parameters.AddWithValue("@contact", txtAdminContact.Text);
                    cmd.ExecuteNonQuery();
             
                    cmd = new SqlCommand(query_2, _isqlDataFunctions.GetConnection());
                    var employeeid = cmd.ExecuteScalar();
     
                    cmd = new SqlCommand(query_3, _isqlDataFunctions.GetConnection());
                    cmd.Parameters.AddWithValue("@email", txtAdminEmail.Text.Trim());
                    cmd.Parameters.AddWithValue("@password", SqlDataFunctions.hashPassword(txtAdminPass.Text.Trim()));
                    cmd.Parameters.AddWithValue("@employeeId", employeeid);
                    cmd.ExecuteNonQuery();

                    _isqlDataFunctions.GetConnection().Close();
                    MessageBox.Show("Your account has been registered succesfully!");
                    clearFields();          
                }
                catch (Exception ex)
                {
                    MessageBox.Show(ex.Message);
                };
            }
        }
   
        private void adminSignInBtn_Click(object sender, EventArgs e)
        {
           
            string query = "SELECT * FROM employeeLogin where email= @email AND password = @password ";

            cmd = new SqlCommand(query, _isqlDataFunctions.GetConnection());
            cmd.Parameters.AddWithValue("@email", adminSignInEmail.Text.Trim());
            cmd.Parameters.AddWithValue("@password", SqlDataFunctions.hashPassword(adminSignInPass.Text.Trim()));
            _isqlDataFunctions.Login(cmd, new adminLogin(), new TGCS_backend.backend());
                       
        }
        private void customerLoginbtn_Click(object sender, EventArgs e)
        {
            var userLogin = new SignIn();
            this.Hide();
            userLogin.Show();
        }
        private void clearFields()
        {
            txtAdminFirstName.Text = "";
            txtAdminLastName.Text = "";
            comboBoxAdminPosition.Text = "";
            txtAdminPass.Text = "";
            txtAdminContact.Text = "";
            txtAdminEmail.Text = "";
        }

        private void ExitBtn_Click(object sender, EventArgs e)
        {
            Environment.Exit(0);
        }
    }

 

[jmcilhinney]

a person told me that i should be using SSMS to write my SQL queries and then paste them back into a string variable in visual studio?

You don't have to do that but it's a good idea to. I could write code directly into this forum and it could be correct, but it's still more error-prone than writing in in VS with the help of the compiler and then testing that it works as expected. Likewise, SSMS will validate your SQL code and you can run it against your database easily to make sure that you get the expected results.

 

[jmcilhinney]

I don't see any obvious issues with your code as far as the SQL and parameters are concerned. There are some other issues but, if your code runs as expected, you can assume that your use of ADO.NET parameters is sound.

 

[jmcilhinney]

One thing I can't help but point out is this:

select employeeId FROM employee WHERE employeeId = (SELECT MAX(employeeId) FROM employee)

Think about what you're actually trying to achieve there and what you're actually doing. The end result will be correct but does that SQL really make sense?

 

[sock1992]

You don't have to do that but it's a good idea to. I could write code directly into this forum and it could be correct, but it's still more error-prone than writing in in VS with the help of the compiler and then testing that it works as expected. Likewise, SSMS will validate your SQL code and you can run it against your database easily to make sure that you get the expected results.

Yeah I'm pretty confident that there are no issues with any of the queries, I've tested them numerous times and they all work as intended. I was just a bit concerned when i was given that comment yesterday, made me panic

 

[sock1992]

One thing I can't help but point out is this:

select employeeId FROM employee WHERE employeeId = (SELECT MAX(employeeId) FROM employee)

Think about what you're actually trying to achieve there and what you're actually doing. The end result will be correct but does that SQL really make sense?

Ah yes, I've been meaning to change that, I'll look into that now

 

[Skydiver]

As I mentioned in the other thread, that is still a terrible way to figure out what ID got assigned to a new user you just inserted into a your database. Consider what happens if two user insertions happen back to back before your query for determining what ID got assigned to the user.

 

[jmcilhinney]

As I mentioned in the other thread, that is still a terrible way to figure out what ID got assigned to a new user you just inserted into a your database. Consider what happens if two user insertions happen back to back before your query for determining what ID got assigned to the user.

Indeed. You should be including a SELECT statement in the same command as your INSERT, using SCOPE_IDENTITY to get the last ID generated in the current scope and then retrieving that value via a parameter. Just like method parameters in C#, SQL parameters are usually used for input but can be used for output or both.