If-Else statement not working within a while loop

Question

I have an issue where I'm trying to create a login form, however the else statement seems to be ignored.

How can I write this code extract so that a message box is shown when the incorrect data is put into the text boxes? (All databases are set up correctly).

try
{
    sc.Open();
    SqlDataReader myReader = null;
    SqlCommand myCommand = new SqlCommand("select * from StudentRecords where ID = '" + txtBoxUsername.Text + "' ", sc); //where ID = '" + txtBoxUsername.Text + "' and DOB = '" + textBoxPassword.Text + "'
    myReader = myCommand.ExecuteReader();

    while (myReader.Read())
    {
        if (txtBoxUsername.Text == (myReader["ID"].ToString()) && textBoxPassword.Text == (myReader["DOB"].ToString()))
        {
            LoginSuccessForm loginfrm = new LoginSuccessForm();
            loginfrm.Show();
            this.Hide();
        }
        else if (txtBoxUsername.Text != (myReader["ID"].ToString()) || textBoxPassword.Text != (myReader["DOB"].ToString()))
        {
            MessageBox.Show("Incorrect Password and/or Username", "Error");
            break;
        }

    }
    sc.Close();
}

I have tried putting the messagebox outside of the while loop and that doesn't work in the desired way. (Following the try method is a catch, I didn't include it to save space).

In saying that, it seems to only pick up the first user in the database too. Any clues or guidance would be appreciated!

You should really be using parameterized queries - you are vulnerable to sql injection here. — Blorgbeard
– Blorgbeard, Commented Apr 29, 2013 at 0:07
Thanks for the reply, however SQL injections are not something I'll be concerned about as this is simply a University project. — Glitchezz
– Glitchezz, Commented Apr 29, 2013 at 0:08
Not saying this is the problem, but you can replace your else if (txt... with a simple else. The check in the if is redundant. — MAV
– MAV, Commented Apr 29, 2013 at 0:12

Blorgbeard · Accepted Answer · 2013-04-29 00:14:26Z

4

You don't need to loop through the results, since you're only expecting one row max. I would do it this way:

using (var cmd = sc.CreateCommand()) {
   cmd.CommandText = "select 1 from Students where Username=.. and Password= ..";
   if (cmd.ExecuteScalar() != null) {
      // username and password matched a user
   }
   else {
      // no match 
   }
}

ExecuteScalar returns the first column of the first row, or null if there was no result.

If this was a real project, you'd need to use SqlParameters to avoid SQL injection vulnerabilities and also look at hashing/salting rather than storing plain-text passwords.

answered Apr 29, 2013 at 0:14

Blorgbeard

104k50 gold badges237 silver badges276 bronze badges

Sign up to request clarification or add additional context in comments.

Collectives™ on Stack Overflow

If-Else statement not working within a while loop

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related