A simple Java login system I created a simple log-in system in Java for a project. Security isn't a concern here, hence the plaintext password, but I'm interested in how I can improve the quality of my code. Everything seems to work but it just feels like messy code.

public boolean logIn(String name, String password)
{
    if (getLoggedInUser() != null)
    {
        throw new IllegalArgumentException("You are already logged in");
    }

    boolean login = false;

    for (User user: getUsers())
    {
        if (user.toString().equals(name) &amp;&amp; user.getPassword().equals(password))
        {
            loggedInUser = user;
            login = true;
        }
    }
    if (!login) //what if the username + password combo is not correct
    {
        for (User user: getUsers())
        {
            if (user.toString().equals(name))
            {
                throw new IllegalArgumentException("Your password is incorrect");
            }
        }
        throw new IllegalArgumentException("Your username is incorrect");
    }
    return login;
}
The Algorithm

The method logIn can be chunked into small steps:


  public boolean logIn(String name, String password) {

   if( /* user is logged in already? */)

   for (/* every user */)
     if ( /* has name and password */)

   if (/* no match found */)
     for (/* every user */)
       if ( /* can find user with the given name? */ )
}



The logic is more complex than it has to be. The algorithm searches twice if a user with the name exists. A simpler algorithm could look like:

public boolean logIn(String name, String password) {
    User user = // find user by its name

    if (/* no user found */)

    if (user.isLoggedIn)

    return user.hasPassword(password)     
}




Data Structure

The method getUsers() looks like it returns a List. Since it is a list and you do not know at which index a concrete user is saved you have to search for the user.


  for (User user: getUsers())
{
  if (user.toString().equals(name))



To check that a user with name does not exists you have to loop through the hole list and this could take some time if you have many users! This is also known as \$O(n)\$ which is a indicator for the time complexity a algorithm can have.

It would be much more performant if we could get a user directly without to search it which would be a time complexity of \$O(1)\$.

We can archive this goal by using a Map instead of a List:

Map&lt;String, User&gt; nameByUser = new HashMap&lt;&gt;();

nameByUser.put("TomZ", new User("TomZ", "aPassw0rt"));
// ..insert some more users

User user = nameByUser.get("TomZ");