0

So im working on a function on a program which should allow you to create a new employee. It takes some parameters, and then adds the userinputs into the database and in the program. my problem is tho that it never gets created in the databse.

I tried running it with a breakpoint, but im not that good at understanding where it goes wrong yet, as it seems to be working fine through all the steps in the method, so im assuming its the query there's something wrong with?

Thank you for taking a look at it and trying to help, it's appriciated

This is the initial form that lets the user write down the different information about the employee.

 public partial class AddAccountDialog : Form
{
    #region Properties

    public Account Account { get; set; }
    public SQLQuery SQL { get; set; }
    public string Id { get; set; }
    public string Firstname { get; set; }
    public string Lastname { get; set; }
    public string Username { get; set; }
    public string Password { get; set; }
    private bool Accepted { get; set; }

    #endregion
    public AddAccountDialog(SQLQuery sql)
    {
        #region Applying Properties

        SQL = sql;
        Accepted = false;


        #endregion

        #region Setup Methods
        InitializeComponent();
        SetupForm();
        ShowDialog();
        #endregion
    }

    private void SetupForm()
    {
        textBox_user_id.Text = Convert.ToString(SQLQuery.GenerateID(100000000, 99999999));
    }

    private void textBox_user_id_TextChanged(object sender, EventArgs e)
    {
        Id = textBox_user_id.Text;
    }

    private void textBox_firstname_TextChanged(object sender, EventArgs e)
    {
        Firstname = textBox_firstname.Text;
    }

    private void textBox_lastname_TextChanged(object sender, EventArgs e)
    {
        Lastname = textBox_lastname.Text;
    }

    private void textBox_username_TextChanged(object sender, EventArgs e)
    {
        Username = textBox_username.Text;
    }

    private void textBox_password_TextChanged(object sender, EventArgs e)
    {
        Password = textBox_password.Text;
    }

    public Account GetResponse()
    {
        Close();

        if (Accepted) return Account;
        else return null;

        Dispose();
    }

    private void button_accept_changes_Click(object sender, EventArgs e)
    {
        Accepted = true;

        if (string.IsNullOrEmpty(textBox_firstname.Text) || 
                string.IsNullOrEmpty(textBox_lastname.Text) ||
                    string.IsNullOrEmpty(textBox_username.Text) ||
                        string.IsNullOrEmpty(textBox_password.Text))
        {
            MessageBox.Show("Udfyld alle felter");
        }
        else
        {
            Account = new Account(
                textBox_firstname.Text, textBox_lastname.Text, textBox_username.Text, textBox_password.Text, Convert.ToString(SQLQuery.GenerateID(10000000, 99999999)), null, false);

            if (new InformationYesNoDialog($"Opret Buger ({Account.FullName}", $"Vil du gerne tilføje {Account.ToString()} til systemet").GetResponse())
            {
                Accepted = true;
                SQL.AddAccount(Account);
                Close();
            }
            else
            {
                Accepted = false;
            }
        }
    }
 
}

This is my SQL query:

 public void AddAccount(Account account)
    {
        string sqlparams = "";
        sqlparams += "server=" + host + ";";
        sqlparams += "user=" + user + ";";
        sqlparams += "database=tier1mtg_login;";
        sqlparams += "port=" + port + ";";
        sqlparams += "password=" + pass + ";";

        MySqlConnection con = new MySqlConnection(sqlparams);

        try
        {
            con.Open();
            MySqlCommand cmd;
            MySqlDataReader rdr;

            #region SQLInsert
            string sqlObject = $"INSERT INTO aaa_user_login VALUES(";
            sqlObject += $"( '{account.UserID}', '{account.Username}', '{account.Password}', '{account.FirstName}' , '{account.LastName}', ";
            #endregion

            cmd = new MySqlCommand(sqlObject, con);
            rdr = cmd.ExecuteReader();
        }
        catch (Exception ex)
        {
            con.Close();
            Console.WriteLine(ex);
        }

     

    }
AnthonLUL
  • 1
  • 1
  • Your query is malformed, i guess. After concatination you have the query string `INSERT INTO aaa_user_login VALUES(( '{account.UserID}', '{account.Username}', '{account.Password}', '{account.FirstName}' , '{account.LastName}', ` With two opening brackets, a trailing comma and no closing bracket. Second thing i recognized, you should use `ExecuteNonQuery` instead of `ExecuteReader` as you are not going to read any data from a result set. – Gabriel Apr 14 '22 at 14:54
  • 1
    Don't write SQLs like that; it enables the biggest, and easiest, hacking risk available these days. Please read https://bobby-tables.com as a matter of urgency and cease writing SQL like that ever again – Caius Jard Apr 14 '22 at 14:54
  • @CaiusJard i would never write queries like this, but the whole program is made like this, and im simply just a little pawn who's suppose to make a couple functions the way they do it haha – AnthonLUL Apr 14 '22 at 15:18
  • Press for change - noone wants to be the dev who wakes up one morning knowing they were the last person to touch bad code that led to the release of [hundreds of thousands of children's private information](https://duckduckgo.com/?t=ffab&q=vtech+hack&ia=web) and feel they could have done something about it.. – Caius Jard Apr 14 '22 at 15:34

1 Answers1

2

It's actually a lot harder to get it wrong if you use parameters because you aren't stopping and starting your string - you're just writing one nice long block of SQL; you should always use parameters:

            string sql = @"INSERT INTO aaa_user_login 
              VALUES(@UserID, @Username, @Password, @FirstName, @LastName)";


            using var con = new MySqlConnection(...);
            using var cmd = new MySqlCommand(sql, con);

            cmd.Parameters.AddWithValue("@UserID", account.UserID);
            cmd.Parameters.AddWithValue("@Username", account.Username);
            cmd.Parameters.AddWithValue("@Password", SecurePasswordHasher.Hash(account.Password)); //see https://stackoverflow.com/questions/4181198/how-to-hash-a-password - never, ever store plaintext passwords
            cmd.Parameters.AddWithValue("@FirstName", account.FirstName);
            cmd.Parameters.AddWithValue("@LastName", account.LastName);

            con.Open(); 
            int r = cmd.ExecuteNonQuery();

If you ever use SQL Server, avoid AddWithValue too; MySql doesn't care/they actively promote its use but it can cause perf issues in SQLS, maybe other databases - check on a per DB basis

Caius Jard
  • 72,509
  • 5
  • 49
  • 80
  • worked. didn't do exactly like the example as i couldn't use "using" but i figured out a way to do it. appriciate it thanks! – AnthonLUL Apr 14 '22 at 23:40
  • If you cannot use `using var x = ...; ...` you should at least be able to use `using(var x = ...){ ... }` – Caius Jard Apr 15 '22 at 04:58