using IF condition inside a while loop in C#

Question

I have a problem with my C# code. I have created a login form in C# 2010. When I am validating the user name, I used an if-condition inside the while loop but the thing is that even when the username and password are correct, it executes the else-statement. Please help me to solve this.

Here is my code :

private void btnlogin_Click(object sender, EventArgs e) {
    string connection=
        @"Data Source=.\SQLEXPRESS;" 
        +" AttachDbFilename=|DataDirectory|ResturantDB.mdf;"
        +" Integrated Security=True; User Instance=True";

    SqlConnection cn=new SqlConnection(connection);

    try {
        cn.Open();
    }
    catch(Exception) {
        // print the exception's message?
        MessageBox.Show("Connection to Database failed; check Connection!");
    }

    SqlCommand cmd=new SqlCommand("SELECT * FROM [Login]", cn);
    cmd.Connection=cn;
    SqlDataReader reader=null;
    reader=cmd.ExecuteReader();

    while(reader.Read()) {
        if(
            txtuser.Text==(reader["Username"].ToString())
            &&
            txtpass.Text==(reader["Password"].ToString())
            ) {
            //MessageBox.Show( "logged in!" );
            Home newhome=new Home();
            newhome.Show();
            this.Hide();
        }
        else {
            MessageBox.Show("Incorrect credentials!");
        }
    }
}

First: it appears that you store passwords in plain text. Don't! There are plenty of questions about password hashing and salting here on SO. please read them carefully. Second: you don't want to query all rows in your Login-Table. Just the one that corresponds to the given Username. Then only check that once. Don't use a loop here. — Corak
– Corak, Commented Apr 26, 2013 at 6:45

hoffmanuel · Accepted Answer · 2013-04-26 06:50:12Z

3

you should use a break, when a username is found in your if condition like

bool found = false;
while (reader.Read())
{   
  if (txtuser.Text == (reader["Username"].ToString()) && txtpass.Text == (reader["Password"].ToString()))
  {
    //MessageBox.Show("loged in!");
    Home newhome = new Home();
    newhome.Show();              
    this.Hide();
    found = true;
    break;
  }
}

if (!found)
    MessageBox.Show("Incorrect credentian..!");

you get into the else block because if any login is not correct, the messagebox appears and that is in n-1 cases in your code.

answered Apr 26, 2013 at 6:50

hoffmanuel

4131 gold badge6 silver badges15 bronze badges

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

2 Comments

Nilan Niyomal Nathanielsz Over a year ago

thank you hoffmanuel its really helped and i did it now its work fine thank you..!!

hoffmanuel Over a year ago

you're welcome. but i suggest to not use plain text passwords as Corak mentioned

Kent Boogaart · Accepted Answer · 2013-04-26 06:46:14Z

2

You're checking if all users have the same user name and password. You need to refine your SQL to select only that one user. Also, please read into password hashing for the sake of your users.

answered Apr 26, 2013 at 6:46

Kent Boogaart

180k39 gold badges401 silver badges398 bronze badges

Comments

Nikhil Agrawal · Accepted Answer · 2013-04-26 09:13:43Z

2

Because its in a loop.

create a bool variable. update its value in loop (if found same username and password) and check outside based on its value.

Do this

bool found;
while (reader.Read())
{
    if (txtuser.Text == (reader["Username"].ToString()) && 
        txtpass.Text == (reader["Password"].ToString()))
    {
        found = true;
        break;
    }                
}
if (found)
{
    MessageBox.Show("loged in!");
    Home newhome = new Home();
    newhome.Show();

    this.Hide();
}
else
{
    MessageBox.Show("Incorrect credentian..!");
}

edited Apr 26, 2013 at 9:13

answered Apr 26, 2013 at 6:46

Nikhil Agrawal

48.8k25 gold badges134 silver badges212 bronze badges

1 Comment

Corak Over a year ago

This would work, if by chance the first found record is the one with the correct username (and password). Better would be "SELECT * FROM [Login] WHERE Username = @Username". And then cmd.Parameters.AddWithValue("@Username", txtuser.text)

TRMI · Accepted Answer · 2013-04-26 07:04:34Z

0

I will solve it on this way:

private void btnlogin_Click(object sender, EventArgs e)
{
    string connection = @"Data Source=.\SQLEXPRESS;AttachDbFilename=|DataDirectory|ResturantDB.mdf;Integrated Security=True;User Instance=True";
    SqlConnection cn = new SqlConnection(connection);
    try
    {
        cn.Open();
    }
    catch (Exception)
    {
        MessageBox.Show("Conncetion to Database faild check Connection !");
    }

    while (true)
    {
        SqlCommand cmd = new SqlCommand("SELECT [Password] FROM [Login] WHERE [Username] = '" + txtuser.Text + "'", cn);
        cmd.Connection = cn;
        SqlDataReader reader = null;
        reader = cmd.ExecuteReader();

        if (!reader.HasRows)
            MessageBox.Show("User does not exist. Please, try again.");
        else
        {
            //username should be unique, so only one row is possible to have
            reader.Read();
            if (txtpass.Text == (reader["Password"].ToString()))
                {
                    //MessageBox.Show("loged in!");
                    Home newhome = new Home();
                    newhome.Show();

                    this.Hide();
                    return;
                }
            else
                    MessageBox.Show("Incorrect credentian..! Try again.");
        }
    }
}

answered Apr 26, 2013 at 7:04

TRMI

1535 bronze badges

1 Comment

Corak Over a year ago

Take a look at the comments to @crapple's answer. Use SqlParameter and as @comecme said, you might run into problems if one Username has several Passwords.

score 0 · Accepted Answer · 2013-04-26 07:11:18Z

0

Simplest and Secure method

 SqlCommand cmd = new SqlCommand("Select uname, pswd from [Login] where uname =@uname and pswd =@ps", conn);
        cmd.Parameters.Add(new SqlParameter("@uname", "username here"));
        cmd.Parameters.Add(new SqlParameter("@ps", "pasword here"));            
        SqlDataReader reader = cmd.ExecuteReader();
        if (reader.Read()) 
        {
             //MessageBox.Show( "logged in!" );
            Home newhome = new Home();
            newhome.Show();

            this.Hide();

        }
        else
        {
            MessageBox.Show( "Incorrect credentials!" );
        }

edited Apr 26, 2013 at 7:11

answered Apr 26, 2013 at 7:06

user1776054

Comments

crapple · Accepted Answer · 2013-04-26 06:50:37Z

-1

No need to loop thru the records for your case use this query, compate username and password in the query:

"SELECT * FROM [Login] where Username='" + txtuser.Text "' and password = '"  + txtpass.Text + "'"

answered Apr 26, 2013 at 6:50

crapple

91 silver badge3 bronze badges

4 Comments

Corak Over a year ago

For the sake of all that's good and holy, please use SqlParameter.

comecme Over a year ago

Theoretically it could be that a user can have multiple passwords.

Corak Over a year ago

@comecme - indeed, but that would not be relevant, because for a login you only want to know if there exists at least one valid login for the provided username and password. If there are more than one, but not that specific one, then login fails.

comecme Over a year ago

@Corak Sorry, I didn't see you also included the password in the query. Only saw you included the Username.

Collectives™ on Stack Overflow

using IF condition inside a while loop in C#

6 Answers 6

2 Comments

Comments

1 Comment

1 Comment

Comments

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

2 Comments

Comments

1 Comment

1 Comment

Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related