5
\$\begingroup\$

As part of a school project for ICT lessons, our class has been required to write a quiz program following certain criteria that has been set. The first part of this is a program that handles the registration process by asking the user for details such as player name, age, email and gender.

import getpass
import os
import pickle
import time

nope = "I didn't quite get that. Mind trying again?"
yn = {'y': True, 'n': False}

if os.path.exists('players'):
    with open('players', 'rb') as f:
        if os.stat('players').st_size == 0:
            accounts = {}
        else:
            accounts = pickle.loads(f.read())
else:
    f = open('players', 'wb')
    f.close()
    accounts = {}

class Player(object):

    def __init__(self):
        self.name = ""
        self.email = ""
        self.password = ""
        self.age = ""
        self.gender = ""

    def write(self):
        accounts[p.name] = [p.email, p.password, p.age, p.gender]
        with open('players', 'wb') as f:
            pickle.dump(accounts, f)

p = Player()

def askChoices(question, options, errorMessage):
    while True:
        answer = str(input(question)).lower()
        if answer in options:
            return options[answer]
        else:
            print(errorMessage)

def register():

    print("\nWelcome, new player!")

    while True:
        p.name = input("What is your name? ")
        if p.name in accounts:
            print("Name already taken. Please try again.")
        else:
            break

    isEmailValid = False
    while not isEmailValid:
        p.email = str(input("What is your email address? "))
        if '@' in p.email and '.' in p.email:
            isEmailValid == True
            break
        else:
            print(nope, "(Input must contain '@' and '.')")

    p.password = getpass.getpass('Please enter a password. ')

    while True:
        try:
            p.age = int(input("How old are you? "))
            break
        except ValueError:
            print(nope, "(Input must be an integer)")

    p.gender = askChoices("What gender are you? ('m' for male, 'f' for female, 'o' for other) ", # it was not my idea to add the 'other' option
                          {'m': "Male", 'f': "Female", 'o': "Other"},
                          nope)

    print("\nName:", p.name,
          "\nEmail:", p.email,
          "\nPassword:", ('*' * len(p.password)),
          "\nAge:", p.age,
          "\nGender:", p.gender)

    correct = askChoices("Is this information correct? (y/n) ",
                         yn, nope)

    if correct:
        print("\nRegistration successful!")
        p.write()
        login()
    elif not correct:
        tryAgain = askChoices("\nWould you like to try again? (y/n) ",
                              yn, nope)
        if tryAgain:
            register()
        else:
            main()

def login():
    while True:
        name = input("What is your player name? ")
        if name in accounts:
            pw = getpass.getpass("Please enter your password. ")
            if pw == accounts[name][1]:
                print(accounts[name]) # debug placeholder until quiz functionality is added
                break
        else:
            print("Account not found.")
            registerNew = askChoices("Would you like to register a new account? (y/n) ",
                                     yn, nope)
            if registerNew:
                register()
                break
            else:
                pass

def main():
    if os.stat('players').st_size == 0:
        register()
    else:
        player = askChoices("Would you like to REGISTER or LOG IN? ",
                            {'register': True, 'log in': False, 'login': False},
                            nope)
        if player:
            register()
        else:
            login()
    print("\nShutting down...")
    time.sleep(3)

if __name__ == '__main__':
    main()

A whole host of different references and web pages had been used to implement some of the features in the program, such as several Stack Overflow posts and even help from regulars of the general Arqade chatroom.

In retrospect, this has deviated strongly from the original examples given in class (and with good reason; the exam board setting this task has really poor coding standards).

I hope I can make this as a fully-working quiz game application at some point.

\$\endgroup\$
4
\$\begingroup\$

Avoid global variables

They are so hard to reason about.

Move p = Player() out of global scope and to the start of register()

And inside the Player class use self:

def write(self):
    accounts[self.name] = [self.email, self.password, self.age, self.gender]
    with open('players', 'wb') as f:
        pickle.dump(accounts, f)

A player should know how to print himself

The following is an implementation detail:

print("\nName:", p.name,
      "\nEmail:", p.email,
      "\nPassword:", ('*' * len(p.password)),
      "\nAge:", p.age,
      "\nGender:", p.gender)

It is more clear to write:

print(p)

To allow this you should implement a __str__ method in the Player class.

Use a function to handle input validation

In-lining all that code for input validation makes the code noisy, and it becomes hard to follow the high level flow of your code, a general_input function may be nice to write:

p.name = general_input(
    "What is your name? ",
    validation = lambda name: name not in accounts,
    error_message = "Name already taken. Please try again."
)

p.email = general_input(
    "What is your e-mail? ",
    validation = lambda email: '@' in email or '.' in email,
    error_message = "(Input must contain '@' and '.')"
)

p.password = getpass.getpass('Please enter a password. ')

p.age = general_input(
    "How old are you? ",
    validation = lambda age: all(i in '1234567890' for i in age)
    error_message = 'Age should be a positive integer'
)

Anything calls anything?

register may call log-in, login may call register, it does not feel clean. User interaction menu functions calling each other freely is just glorified go-to.

I suggest that only main may call login and register and they may not call each other, but just main back. It would simplify the code and maybe even make the program more predictable to use when/if it grows (When you are unsure about how to do an action, just go back to the main menu and you will be shown how)

For example look how simple login becomes when you make it jump back unconditionally to main:

def login():
    while True:
        name = input("What is your player name? ")
        if name not in accounts:
            print("Account not found.")
            main()

        pw = getpass.getpass("Please enter your password. ")
        if pw == accounts[name][1]:
            print(accounts[name]) # debug placeholder until quiz functionality is added

Tiny helper functions

Do not feel bad for writing tiny functions that make your code even just a little more readable, they are good! For example:

os.stat('players').st_size == 0

Takes some time to understand, reading is not fluent, but if I define:

def is_file_empty(filename):
    return os.stat('players').st_size == 0

Reading is_file_empty('players') is instantaneous.

The same goes for:

f = open('players', 'wb')
f.close()

Opening a file and closing it again is weird... a small aptly named function will make your intent much clearer.

\$\endgroup\$
  • \$\begingroup\$ Oof. I didn't even notice that I used p instead of self. And you bring about some valid points in terms of readability. \$\endgroup\$ – nine Dec 2 '15 at 4:45
  • \$\begingroup\$ Hey, uh, so unfortunately I'm unable to firgure out how I should be implementing the general_input function. Would you be able to give some pointers on that? \$\endgroup\$ – nine Dec 2 '15 at 18:42
  • \$\begingroup\$ @Texenox Are you familiar with first class functions? \$\endgroup\$ – Caridorc Dec 2 '15 at 18:58
  • \$\begingroup\$ ...I'm not sure. In fact, it seems like I may have forgotten a tag for this question from the start. \$\endgroup\$ – nine Dec 2 '15 at 18:59
  • 1
    \$\begingroup\$ @Texenox Yeah, the age example was from a more general function :) Just for comparison, I wrote the function like this: pastebin.com/LPxKjzHJ \$\endgroup\$ – Caridorc Dec 2 '15 at 19:05
3
\$\begingroup\$

Creating an empty Player then filling it externally isn't a great idea - I would be inclined to use a class method as an alternate constructor, keeping all of the player-related logic in one place, for example:

class Player(object):

    def __init__(self, name, email, password, age, gender):
        self.name = name
        self.email = email
        self.password = password
        self.age = age
        self.gender = gender

    @classmethod
    def from_input(cls):
        # ... take user input
        return cls(name, email, password, age, gender)

Then you can use:

p = Player.from_input()

Try to reduce duplication of information. For example:

askChoices("What gender are you? ('m' for male, 'f' for female, 'o' for other) ",
           {'m': "Male", 'f': "Female", 'o': "Other"},
           nope)

You have the same information in the first and second parameters; why not build the string using the options. You're also mixing single (') and double (") quotes; try to be consistent. You could make nope (which is a constant, so should be in UPPERCASE) a default parameter value, to avoid passing it every time. A much neater call would then be:

askChoices("What gender are you?",
           {"m": "Male", "f": "Female", "o": "Other"})

Be consistent in your control flow. Compare:

while True:
    p.name = input("What is your name? ")
    if p.name in accounts:
        print("Name already taken. Please try again.")
    else:
        break

with:

isEmailValid = False
while not isEmailValid:
    p.email = str(input("What is your email address? "))
    if '@' in p.email and '.' in p.email:
        isEmailValid == True
        break
    else:
        print(nope, "(Input must contain '@' and '.')")

The second has an obviously unnecessary variable, given that you break as soon as the variable changes anyway. Also, your validation permits '@.' as an email address.

\$\endgroup\$
  • \$\begingroup\$ Huh. I actually forgot about getting rid of isEmailValid. \$\endgroup\$ – nine Dec 2 '15 at 4:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.