Why would this code display error even when it shouldn't?-CodePudding

I am sort of new to programming and I cannot get this simple looking code to work What I am trying to do is make a login system that works on an array so its easier to edit and add new people onto the list.

string[] UserCodes = { "admin", "testcode" }; //these are the arrays
string[] UserWords = { "123", "testword" };

public void Button_Clicked(object sender, EventArgs e)
{
    for (int i = 0; i < UserCodes.Length; i  )
        if (UserCode.Text.Equals(UserCodes[i])  && UserWord.Text.Equals(UserWords[i]))
        {
            Navigation.PushAsync(new HomePage());
        }
    for (int i = 0; i < UserCodes.Length; i  )    
        if (UserCode.Text != (UserCodes[i]) || UserWord.Text !=(UserWords[i]))
        {
            DisplayAlert("Something Went Wrong", "Incorrect Password or Username", "Try Again");
        }   
}                   // And this is the "main" code

I believe the main problem arises from the second part, where it displays an error, because if I don't enter the password correctly it works as it should but if I do enter correctly It sends me to the home page while still displaying the error. I have tried using else if, and i tried to use (!command.Equals(Array[i])). I am extremely confused as to why its acting this way.

CodePudding user response：

Step through this code in the debugger. If you type in a correct UserCode and UserWord you will call Navigation.PushAsync(new HomePage())... and then keep going.

You next check if any UserWord or UserCode is not what you typed, and it will be, then show your error message.

You probably want to call return after Navigation.PushAsync(new HomePage()), or skip over the second for statement.

CodePudding user response：

It feels like a code smell to me when you have one loop checking for a condition followed by an entirely different loop for when that condition fails. As @Corvus noted, you should return once you've made a match, and then you defer the error message after all looping is complete:

public void Button_Clicked(object sender, EventArgs e)
{
    for (int i = 0; i < UserCodes.Length; i  )
    {
        if (UserCode.Text.Equals(UserCodes[i]) &&  UserWord.Text.Equals(UserWords[i]))
        {
            Navigation.PushAsync(new HomePage());
            return; // thanks to @Corvus
        }
    }
    DisplayAlert("Something Went Wrong", "Incorrect Password or  Username", "Try Again");
}

That said, there are improvements you could make but did not ask about. Usually, the user name is acceptable regardless of case, but the password always must match the case. You are okay on the treatment of passwords but the user name match could be changed to ignore case.

Also, you have two arrays that must be in-sync with each other. I would suggest instead a Dictionary<string, string> where the user name is the Key and password is the Value.