6
\$\begingroup\$

I've been learning Python for a few weeks and I created a simple game where you convert binary numbers to decimal. It shows you n binary numbers and you are supposed to enter the numbers in decimal system in sequence as a password. After failing three times you lose. I'd like to implement it into a bigger game later.

Here's the code:

import random


# Conver decimal to binary.
def toBin(i):
    bin = "{0:b}".format(i)
    return bin


# Generate random password and it's binary version.
def generatePassword(n):
    password = []
    passwordBin = []
    for i in range(n):
        a = random.randint(0, 9)
        password.append(a)
        passwordBin.append(toBin(a))
    return password, passwordBin


# Prints the binary password and a frame.
def printPassword(password):
    print(12 * '#')
    for i in range(len(password[1]) + 4):
        if i <= 1:
            print(2 * '#' + 8 * ' ' + 2 * '#')
        elif i <= 5:
            print(2 * '#' + password[1][i - 2].rjust(4, '0').center(8) + 2 * '#')
        else:
            print(2 * '#' + 8 * ' ' + 2 * '#')
    print(12 * '#')


# The game loop.
def puzzle(n):
    password = generatePassword(n)
    win = False
    endLoop = False
    attempts = 3
    while endLoop is False:
        # Check remaining attempts.
        if attempts > 0:
            printPassword(password)
            print('Attempts remaining: ' + str(attempts) + '\n')
            # Looping through password numbers.
            for i in range(n):
                print('Number ' + str(i + 1), '\t\t', end='')
                # Input and check for ValueError.
                try:
                    inp = int(input())
                except ValueError:
                    print('You can only enter numbers.')
                    break
                # Is input correct?
                if inp == password[0][i]:
                    print('CORRECT')
                else:
                    attempts -= 1
                    print('WRONG\n')
                    break
            # End loop and win.
            else:
                endLoop = True
                win = True
        # End loop.
        else:
            endLoop = True
    # Check win condition.
    if win is True:
        print('ACCESS GRANTED')
    else:
        print('ACCESS DENIED')


# Run the game.
puzzle(4)

Are there any ways to improve my code, expecially the puzzle(n) function? The conditions and loops took me some time to get working and still kinda confuse me. I hope there aren't any bugs.

\$\endgroup\$
5
\$\begingroup\$

Style

Please follow PEP8 conventions on naming: function names should be lowercase_with_underscores

On documentation: information on functions should be put below their definition in the form of a docstring, in order to allow programmatic access via help(function).

generatePassword

List comprehension

The explicit loop in generate_password is unnecessary, it is too long making the reader forget about the logic behind such function.

def generate_password_and_binary(n):
    """ A "password" is a list of length `n` containing random digits. """ 
    password = [random.randint(0, 9) for _ in range(n)]
    return password, [to_bin(digit) for digit in password]

This version with list comprehension is shorter and more to the point.

I also incorporated the comment into the function name (self-documenting code is better than code + comments) and added a more detailed docstring.

printPassword

I think your printPassword has a serious bug:

>>> generatePassword(8)
([2, 1, 7, 6, 4, 5, 7, 3], ['10', '1', '111', '110', '100', '101', '111', '11'])
>>> printPassword(_)
############
##        ##
##        ##
##  0010  ##
##  0001  ##
##  0111  ##
##  0110  ##
##        ##
##        ##
##        ##
##        ##
##        ##
##        ##
############

Only the first four digits where printed and the rest ignored. This bug is probably a result of over-complication caused by indexes and comparison, while a plain for iteration over the collection is sufficient, here is a simpler (and correct!) way to write this function:

def print_password(binary_version): # No need to pass all password if only binary part is shown
    wall = "#" * 12
    spaced = "{0}{1}{0}".format("##", " " * 8)
    print(wall, spaced, sep="\n")
    for binary_digit in binary_version:
        print("{0}{1}{2}{1}{0}".format("##", "  ", binary_digit.rjust(4, "0")))
    print(spaced, wall, sep="\n")

Now all of the password is printed:

>>> print_password(['10', '1', '111', '110', '100', '101', '111', '11'])
############
##        ##
##  0010  ##
##  0001  ##
##  0111  ##
##  0110  ##
##  0100  ##
##  0101  ##
##  0111  ##
##  0011  ##
##        ##
############
\$\endgroup\$
2
  • 1
    \$\begingroup\$ I have started thinking about how to simplify the last of your functions: look here: pastie.org/10955242 \$\endgroup\$
    – Caridorc
    Nov 3 '16 at 15:26
  • \$\begingroup\$ Thanks for answer, I found that bug when I was messing around with it later in train. I solved it differently, but your solution seems nicer. \$\endgroup\$
    – Kid B
    Nov 4 '16 at 9:52
4
\$\begingroup\$

You don't need passwordBin as it is only the representation of an information you already have. And, as such, you don't need toBin either. You only need generatePassword to return a list of random digits:

def generate_password(n):
    return [random.randint(0, 9) for _ in range(n)]

and then, format these numbers into printPassword:

def print_password(digits):
    print('#'*12)
    print('##        ##')
    for digit in digits:
        print('##  {:04b}  ##'.format(digit))
    print('##        ##')
    print('#'*12)

You can also reduce the complexity of your main loop by:

  1. Using a decreasing range: for remaining in range(3, 0, -1):
  2. Extracting out the "printing" part and having this function only return if the player wins or fails.

Something along the lines of

# The game loop.
def puzzle(n):
    password = generate_password(n)
    for attempts in range(3, 0, -1):
        print_password(password)
        print('Attempts remaining:', attempts, '\n')
        win = True
        # Looping through password numbers.
        for i, digit in enumerate(password, 1):
            guess = input('Number {}\t\t'.format(i))
            try:
                guess = int(guess)
            except ValueError:
                print('You can only enter numbers.')
                win = False
                break
            if guess != digit:
                print('WRONG!')
                win = False
                break
            print('CORRECT')
        if win:
            # Every guess were the same as the digits
            return True
    # Attempts exhausted without winning
    return False

You may note the use of the prompt parameter of input that avoid a call to print right before; and the use of enumerate that allow to access both the digit and its index at once. The parameter 1 is there to indicate that the indexing start at 1.

Now you could write a game function allong the lines of

def game(n):
    if puzzle(n):
        print('ACCESS GRANTED')
    else:
        print('ACCESS DENIED')

or put this kind of logic under an if __name__ == '__main__': clause.


The use of the win flag can also be simplified further by using the for ... else construct:

def puzzle(n):
    password = generate_password(n)
    for attempts in range(3, 0, -1):
        print_password(password)
        print('Attempts remaining:', attempts, '\n')
        for i, digit in enumerate(password, 1):
            guess = input('Number {}\t\t'.format(i))
            try:
                guess = int(guess)
            except ValueError:
                print('You can only enter numbers.')
                break
            if guess != digit:
                print('WRONG!')
                break
            print('CORRECT')
        else:
            # Every guess were the same as the digits
            return True
    # Attempts exhausted without winning
    return False

It can also be simpler (no need to catch an exception) to turn the digits into strings instead of trying to turn the guesses into integers. In the end, I would write something along the lines of:

import random


def generate_password(n):
    """Generate a list of n random digits"""
    return [random.randint(0, 9) for _ in range(n)]


def print_password(digits):
    """Print a list of digits by obfucating their decimal
    representation into binary one.
    """
    print('############')
    print('##        ##')
    for digit in digits:
        print('##  {:04b}  ##'.format(digit))
    print('##        ##')
    print('############')


def puzzle(n, max_attempts=3):
    """Ask the user to guess a list of n random digits
    by providing them with their binary representation.

    Return whether they win or not.
    """
    password = generate_password(n)
    for attempts in range(max_attempts, 0, -1):
        print_password(password)
        print('Attempts remaining:', attempts, '\n')
        for i, digit in enumerate(password, 1):
            guess = input('Number {}\t\t'.format(i))
            if guess != str(digit):
                print('WRONG!')
                break
            print('CORRECT')
        else:
            # Every guess were the same as the digits
            return True
    # Attempts exhausted without winning
    return False


if __name__ == '__main__':
    user_won = puzzle(10)
    if user_won:
        print('ACCESS GRANTED')
    else:
        print('ACCESS DENIED')
\$\endgroup\$

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.