Login Script Utilising List Comprehension

Question

This a fairly basic script I made for use in some other projects, nothing too complex or sophisticated and obviously lacking in some features (Password/Username encryption, Missing file contingencies etc.).

It loads a text file (formatted as USERNAME, PASSWORD\nUSERNAME, PASSWORD\n) into a 2D array, then checks the inputs against it, until 2 separate logins are used.

---

I'm what I'd consider intermediate in terms of python competency, but there are always things to be improved upon: any suggestions? I tend to use something similar to this for all projects that require a login, so any improvements or areas to look into, no matter how minor, would be greatly appreciated.

---

def login():
    player_count, details = 0, [[data.strip() for data in line.split(",")] for line in open("username_password.txt", "r").read().splitlines()]
    while player_count != 2:
        username, password = input("USERNAME"), input("PASSWORD")
        for data in details:
            if username == data[0] and password == data[1]:
                print("LOGIN CORRECT")
                details.pop(details.index(data))
                player_count += 1
                break
        else:
            print("INCORRECT")
    main()

def main():
    print("LOGGED IN\nMAIN")

login()

Answer 1

I would split off player_count = 0 to a different line. Multiple assignments on one line is perfectly fine for simple assignments (like a, b = 1, 2), but a double list comprehension doesn't qualify for "simple".

Readability Counts

In general, Readability is far more important than conciseness. In fact, the reason concise code is often better is because it's more readable.

Normally, I'd be very careful with double list assignments for that reason. However, in this specific case, I think making it a single line is a good call. It's not overly long nor complex.

Performance

For pure python, I don't think you can really improve your performance even more. You won't get any issues until your password file grows into the MB's, and if that's a problem, you shouldn't be using a file with plaintext passwords anyway, since you'll be running your software in a production environment.

Instead, you'd be hashing and salting your passwords and storing them in a database, which is coincidentally also the best way to improve your performance if you get datasets that large.

However, as long as your username and password are requested from the user by means of the input() function, you won't have an issue.

dict.pop()

This function removes the value from the list. However, you don't seem to be actually doing anything with that value, and you're also discarding the list. So I'd just drop that line.

The only thing I can imagine you fix it would be to stop player2 from using the same credentials as player1. However, you don't save player references anyway. If you want to stop the same credentials from being used twice, I'd recommend to instead save references to the players for later, and then check that a player hasn't already been logged in as another player.

Answer 2

Advises for optimization:

• open("username_password.txt", "r").read(). This could be called as "negligent treatment" of the file object. The good and robust way is to always close the file object/resource after usage/consumption.
  We have a convenient feature called context managers (with statement) for "automated" graceful closing of used resource:

  with open('filename') as f:
      content = f.read()
  

• details = [[...]]. Instead of manually stripping and storing the file contents as list of lists, the more flexible way is using csv.reader object which allows strip of surrounding whitespaces (skipinitialspace=True option) and recognize delimiters.

• considering that your file represents pairs of USERNAME, PASSWORD, in scope of application we usually expect the usernames to be unique. Therefore having a dict indexed with usernames and passwords as respective values would be an optimized and more performant way - giving us a quick search by usernames (Python dict is a special case of hashtable data structure).
  Besides, it's really not good to remove items from the a list in-place while it's iterated (for data in details: if ... details.pop(details.index(data))), it'll lead to unexpected results. Yes, you have a "circuit breaker" break for that, but still it's better to not get used to fragile approaches.

  with open("username_password.txt") as f:
      creds = {u: p for u, p in csv.reader(f, skipinitialspace=True)}
  

• dict.get() call allows to flexibly combine check for username/password:

  if creds.get(username) == password:
      ...
  

---

The final optimized version:

import csv

def login():
    player_count = 0
    with open("username_password.txt") as f:
        creds = {u: p for u, p in csv.reader(f, skipinitialspace=True)}

    while player_count != 2:
        username, password = input("USERNAME"), input("PASSWORD")
        if creds.get(username) == password:
            print("LOGIN CORRECT")
            player_count += 1
            creds.pop(username)
        else:
            print("LOGIN INCORRECT")
    main()


def main():
    print("LOGGED IN\nMAIN")


login()

Answer 3

Here's a small suggestion -

You could make use of the -

if __name__ == "__main__": guard

Like this -

def login():
    player_count, details = 0, [[data.strip() for data in line.split(",")] for line in open("username_password.txt", "r").read().splitlines()]
    while player_count != 2:
        username, password = input("USERNAME"), input("PASSWORD")
        for data in details:
            if username == data[0] and password == data[1]:
                print("LOGIN CORRECT")
                details.pop(details.index(data))
                player_count += 1
                break
        else:
            print("INCORRECT")
    main()

def main():
    print("LOGGED IN\nMAIN")

if __name__ == "__main__":
    login()

Answer 4

I tend to use something similar to this for all projects that require a login

Then it's time to stop using self-rolled unencrypted authentication. It's fine as a one-off proof-of-concept, but as you've described, that's not how it's actually being used.

Read through https://stackoverflow.com/questions/7014953/i-need-to-securely-store-a-username-and-password-in-python-what-are-my-options - specifically, consider using something like keyring. Not only will it be more secure - it will also simplify your code.