Using IF condition inside a while loop in C#. False value still looping

Question

I don't know why my else value inside my while loop is still looping, whether the result is login or invalid. The alertdialog is always pop-up.

These are my codes.

MySqlConnection conn = new MySqlConnection();    
string query = "server=sample.com;port=3306;database=sample;user id=sample;password=sample";
conn.ConnectionString = query;

MySqlCommand cmd = new MySqlCommand("select * from wp_users", conn);
try
{
    conn.Open();


    MySqlDataReader myReader = cmd.ExecuteReader();

    string user1 = "";
    string pass1 = "";
    //bool stopLoop = false; // stop looping for false value;
    while (myReader.Read())
    {

        user1 = myReader[1].ToString(); //datacolumn -> user_login
        pass1 = myReader[2].ToString(); //datacolumn -> user_pass

        if ((user1 == txtUsername.Text) && (pass1 == txtPassword.Text))
        {

            Intent myIntent;
            myIntent = new Intent(Activity, typeof(index));

            string a = user1;

            myIntent.PutExtra("myItem", a);
            StartActivity(myIntent);
        }
        else
        {
            Android.Support.V7.App.AlertDialog.Builder alert = new Android.Support.V7.App.AlertDialog.Builder(Activity);
            alert.SetMessage("Invalid username or password");
            alert.SetPositiveButton("Ok", (senderAlert, args) =>
            {
                alert.Dispose();
            });
            alert.Show();
        }
    }
    myReader.Close();
}
catch (MySqlException ex)
{
    Android.Support.V7.App.AlertDialog.Builder except = new Android.Support.V7.App.AlertDialog.Builder(Activity);
    except.SetTitle("Please report this to our website(error server timeout)");
    except.SetMessage(ex.ToString());
    except.SetPositiveButton("Ok", (senderAlert, args) =>
    {
        except.Dispose();
    });
    except.Show();
}
finally
{
    conn.Close();
}

I tried it insert in my else statement. But the result is always false. But if i remove the break; in my else statement. The user will be login but the alert pop up will show first before the system is login. — Jayson E Garcia, Apr 13 '18 at 06:37
The false alert dialog will pop up first. I dont know how to break it :( — Jayson E Garcia, Apr 13 '18 at 06:39
Reference the column by its name to make sure the column you are checking is what you want to ckeck `myReader["user_login"]`. As a general rule, doing `select * from` is not a good practice, is harder to mantain, if you need extra data from other tables you will retrieve far more data than desired. Finally, you could use the SQL `where` clause and get rid of the while. — Cleptus, Apr 13 '18 at 06:57

score 0 · Answer 1 · answered Apr 13 '18 at 07:07

In your question you never say it to stop reading the datareading nor using a break when the desired record is found.

                MySqlDataReader myReader = cmd.ExecuteReader();

                string user1 = "";
                string pass1 = "";
                bool stopLoop = false;
                while (stopLoop == false && myReader.Read())
                {

                    user1 = myReader[1].ToString(); //datacolumn -> user_login
                    pass1 = myReader[2].ToString(); //datacolumn -> user_pass

                    if ((user1 == txtUsername.Text) && (pass1 == txtPassword.Text))
                    {
                        stopLoop = true;

                        //rest of the true if code

                        // If you didnt use a stopLoop flag you could have inserted here a break;
                    }
                    else
                    {
                        // your else code
                    }
                }
                myReader.Close();

You could also check my suggestions on the question comments to try to make it better, because I believe the while is not necessary ;-)

score 0 · Answer 2 · answered Apr 13 '18 at 07:23

The problem is your logic is slightly flawed let me demonstrate with things. Lets say you have user1, user2, user3 etc. say to 10..

You have

while reading through list
  if entered user = user item from list
     do happy stuff
  else 
     do unhappy stuff
end while.

So i enter user3

if user3 = user1 .. nope say unhappy stuff
if user3 = user2 .. nope say unhappy stuff
if user3 = user3 .. yes do happy stuff
if user4 = user3 .. nope say unhappy stuff
....

if you had 2000 users you'd have 1999 unhappys done.

You need a loop that works like this

item found = false
while reading through list
  if entered user = user from list
   item found = true
    do happy stuff
    break; // because theres no point checking another 1999 !
end while

if item found = false do unhappy stuff

score 0 · Answer 3 · answered Apr 13 '18 at 07:24

There are 2 problems with the code that relate to your issue.

First of all, you do not break the loop when the login is successful. This can easily be done using the break; keyword.

So you would get:

    [...]
    myIntent.PutExtra("myItem", a);
    StartActivity(myIntent);
    break;                               // <----
}
else [...]

The second problem is that in each loop you also handle the case if the login was not correct. Meaning that if the data would not match the data of the first user returned by the database, the code would immediately state that the login failed.

You'll need to track if the input matches any of the users and only if it matches none of them (so after the while loop) you can do the handling of a failed login.

As for some suggestions:

If this will be an application with actual users, please do not store passwords as plain-text in a database, hash them and compare the hashes. See this post on how to hash passwords.

Instead of getting the entire user table from the database and checking each user within a loop, use the query to immediately check for the user you're looking for. Databases are optimized for searching and will make you code much faster.

Something like:

    using (var cmd = new MySqlCommand("select user_login from wp_users where user_login = @user and user_pass = @pass", conn))
    {
        conn.Open();

        // Set parameters
        cmd.Parameters.AddWithValue("@user", user);
        cmd.Parameters.AddWithValue("@pass", pass);

        // Get result
        using (var reader = cmd.ExecuteReader(CommandBehavior.SingleRow))
        {
            // If ANY row, there is a user
            if (reader.Read())
            {
                // Login succesfull
            } 
            else
            {
                // Login failed
            }
        }
    }

Sagar · Accepted Answer · 2018-04-13T06:50:14.407

-1

You should use String.Equals(user1 ,txtUsername.Text.ToString ()) and String.Equals(pass1 ,txtPassword.Text.ToString ()) instead of user1 == txtUsername.Text and pass1 == txtPassword.Textin your if. To have correct string comparison

You should move else part outside of your loop. If username and password are correct then add break; as your last statement in if condition.

You can maintain a boolean variable lets say isLogin=false; Set this variable to true when login is successful. After loop, check the flag and decide whether to show login error dialog.

edited Apr 13 '18 at 06:50

answered Apr 13 '18 at 06:41

Sagar

20,467
4
50
56

Thank you for helping me. I tried your solution, but the result is still same :(. – Jayson E Garcia Apr 13 '18 at 06:47
I don't know why my else statement is always pop up :(. I know there is something wrong in my condition or in my while loop :(. – Jayson E Garcia Apr 13 '18 at 06:48
1

He was already using a `stopLoop` flag, but had it commented. – Cleptus Apr 13 '18 at 07:09

Using IF condition inside a while loop in C#. False value still looping

4 Answers4