how to make login system C#

Question

i have problem with my codes here regarding with login system. when i add 1 username and password in my database, it work properly. but when i add another 1 username and password in database, my else statement will pop up twice. when 3 username and password my else statement will pop up trice. so on and so for.. here's my code.. if you have there better code than mine, please show it.

SqlConnection con = new SqlConnection("Data Source=.\\SQLEXPRESS;AttachDbFilename=C:\\Users\\JEDMARC\\Desktop\\VS v1.0.0\\Voting System v1.0.0\\Voting System v1.0.0.mdf;Integrated Security=True;User Instance=True");
    SoundPlayer t = new SoundPlayer(@"C:\Users\JEDMARC\Documents\welcome.wav");

    private void btnEnter_Click(object sender, EventArgs e)
    {
 if (cmbToE.Text == "HomeRoom Election" && comboBox1.Text == "English")
        {
            con.Open();

            SqlCommand da = new SqlCommand("SELECT * FROM RegistrationTable", con);

            SqlDataReader reader = null;
            reader = da.ExecuteReader();
            while (reader.Read())
            {
                if (tbUsername.Text == (reader["Username"].ToString()) && tbPassword.Text == (reader["Password"].ToString()))
                {
                    MessageBox.Show("*Choose your best candidate. Use a Combobox.\n\n*After choosing, click Submit button to pass your vote!\n\n                           VOTE WISELY!", "How to vote?", MessageBoxButtons.OK, MessageBoxIcon.Asterisk);
                    UserHRForm x = new UserHRForm();
                    x.Show();
                    t.Play();
                    this.Close();
                }
                else
                {
                    SystemSounds.Hand.Play();
                    MessageBox.Show("Access Denied! Account doesn't exist!", "Error", MessageBoxButtons.OK, MessageBoxIcon.Hand);                        
                }
            }
        }
}

Answer 1

To fix the immediate problem you want to use the break keyword to exit the while loop.

Long term you should look at filtering out the users in you SQL call:

SqlCommand da = new SqlCommand(
    "SELECT * FROM RegistrationTable" + 
    "Where Username = @Username and Password = @Password", con);

da.Parameters.Add("@Username ", SqlDbType.Varchar);
da.Parameters["@Username "].Value = tbUsername.Text;
da.Parameters.Add("@Password", SqlDbType.Varchar);
da.Parameters["@Password"].Value = tbPassword.Text;

This will ensure the data reader either returns either one or zero elements (unless someone can duplicate their login). What you are doing is returning the entire table, and looping through each result, SQL can perform this job a lot more efficiently than your C# code. Have a read of “Never do in code what you can get the SQL server to do well for you” - Is this a recipe for a bad design?.

Answer 2

Your while loop checks every user in the DB, so it hits the right user one time and the wrong user twice. To do it the way your code is written now, you would have to use a flag in your while loop to track if you found the right user/password combination. Something like:

        bool foundUser = false;

        while (reader.Read())
        {
            if (tbUsername.Text == (reader["Username"].ToString()) && tbPassword.Text == (reader["Password"].ToString()))
            {
                foundUser = true;
                break;
            }
        }


        if (foundUser) {
                MessageBox.Show("*Choose your best candidate. Use a Combobox.\n\n*After choosing, click Submit button to pass your vote!\n\n                           VOTE WISELY!", "How to vote?", MessageBoxButtons.OK, MessageBoxIcon.Asterisk);
                UserHRForm x = new UserHRForm();
                x.Show();
                t.Play();
                this.Close();
            }
            else
            {
                SystemSounds.Hand.Play();
                MessageBox.Show("Access Denied! Account doesn't exist!", "Error", MessageBoxButtons.OK, MessageBoxIcon.Hand);                        
            }

But that's a terribly inefficient use of code. Instead, let SQL handle the filter for you. That's what it's designed for.

        con.Open();

        SqlCommand da = new SqlCommand("SELECT * FROM RegistrationTable WHERE Username=@Username AND Password=@Password", con);
        da.Parameters.Add("@Username ", SqlDbType.Varchar);
        da.Parameters["@Username "].Value = tbUsername.Text;
        da.Parameters.Add("@Password", SqlDbType.Varchar);
        da.Parameters["@Password"].Value = tbPassword.Text;

        SqlDataReader reader = null;
        reader = da.ExecuteReader();

            if (da.HasRows)
            {
                MessageBox.Show("*Choose your best candidate. Use a Combobox.\n\n*After choosing, click Submit button to pass your vote!\n\n                           VOTE WISELY!", "How to vote?", MessageBoxButtons.OK, MessageBoxIcon.Asterisk);
                UserHRForm x = new UserHRForm();
                x.Show();
                t.Play();
                this.Close();
            }
            else
            {
                SystemSounds.Hand.Play();
                MessageBox.Show("Access Denied! Account doesn't exist!", "Error", MessageBoxButtons.OK, MessageBoxIcon.Hand);                        
            }
        }