HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMajor

Pythons Around the Rose

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
aroundthepythonsrose

Problem

Based off my Java Petals Around the Rose, I thought it would be good practice to write something again in a different language.

Because the language is Python, I named it "Pythons Around the Rose", and everywhere in my code, the word "rose" would be replaced with "python".

Also, "Potentate" is replaced with "Pytentate".

Code:

```
import random

def display_help():
example = get_dice_result()
print("The name of the game is Pythons Around the Rose.",
"The name is important. I will roll five dice,",
"and I will tell you how many pythons there are.",
"\nFor example:", sep = "\n")
print_dice(example)
print("Will result in ", get_answer(example), ".", sep = "")
print("\nIf you answer correctly 8 times in a row, you",
"will be declared a \"Pytentate of the Rose\".", sep = "\n")

def play():
streak = 0
while True:
dice = get_dice_result()
if play_once(dice):
streak += 1
else:
streak = 0
if streak == 8:
print('You are now declared a "Pytentate of the Rose"!')
break;
print("Thank you for playing!")

def get_dice_result():
result = [get_dice_roll(), get_dice_roll(),
get_dice_roll(), get_dice_roll(), get_dice_roll()]
return result

def get_dice_roll():
return random.randrange(6) + 1

def play_once(results):
print("How many pythons here?")
print_dice(results)
guess = get_guess()
answer = get_answer(results)
if guess == answer:
print("Correct!")
return True
print("Incorrect. The answer is ", answer, ".", sep = "")
return False

def get_guess():
guess = 0
valid = False
while not valid:
try:
guess = int(input("> "))
valid = True
except:
print("\nOops! That is not a number. Try again: ")
return guess

def get_answer(dice):
answer = 0
for i in dice:
if i == 3:
answer

Solution

In general your code looks nice, but it does show that you've got a another programming language background, and thus you do it the standard imperative way instead of utilizing some of the more pythonic way. In this review I'm deliberately going a little over the top, but in return you get to see some of the more special pythonic constructions.

-
Using __doc__ to get the docstring for display_help – In Python you can and should describe your code using docstrings, typically text with triple quotes, i.e. """ ... """. These can be used to describe the module, functions, classes (and methods). They typically come on the line after the definition of the item you want to document.

A neat thing is that this docstring can be accessed using a special variable, __doc__, or for a function as display_help.__doc__. And then it can be used for print outs, or similar.

-
Work a little more on naming methods – It is not very common in Python to have get_xxxx methods, rather name them according to specific function, like roll_multiple_dice() or roll_die(). Aim for simplicity and clarity. Using get_answer() implies that you want me to insert the answer of something, whilst number_of_pythons() indicates that this is going to be calculated.

-
List comprehensions are your friend – Instead of repeating code to build the list, use return [roll_die() for _ in range(number_of_dice)] which implements three nice features:

  • A list comprehension, which builds a list based on the inner generator, i.e. [ ... ]



  • The use of for within the generator, x for x in range(y), which compresses a for loop into the expression. This one would return the numbers from 0 to y-1



  • The use of _ when we don't really care about the number, but just want the code in front of the for loop to be executed



-
Generator expressions – The for loop of last item, can also be used outside of list comprehension, i.e sum(x for x in range(y)), which would sum all the numbers from 0 through y-1.

One major difference between a list comprehension and generator, is that the list comprehension actually builds the entire list, whilst the generators works in the background and gives you another element when you want it.

In the sum example this means that if you had y=1000000 you wouldn't allocate memory for more than 1 int, whilst if you used list comprehension you would allocate the entire list of 1 million elements, and then sum it.

-
The Python ternary, i.e. a = b if c else d – This could take some time to get used to, but actually it reads out quite nicely: Set the value of a to b if the condition c is true, or d if not true.

Disclaimer: In my refactored code this is where it got a little ugly, as I combined multiple of this construction. But still, they kind of make sense. I used them in both number_of_pythons() and print_dice().

-
Avoid one-time intermediate variable – It's quite common to skip the intermediate variable, especially if you use them only once, and rather use the expression directly. I.e. in roll_die() just return the random number, and in roll_multiple_dice() return the list directly.

-
In some cases, you can sum using Booleans – Doing the if True: a += 1 is kind of an anti-pattern, and can in some cases be replaced by a += True, or in your case: streak += play_once(...).

  • Bug found by 200_success: Here the simplification went too far, as I by simply summing, removed the resetting of the streak counter.



-
Avoid flag variables, if possible – Your use of valid in get_guess() is better written using while True and a break like in your input validations. And skipping the intermediate variable, you could return straight out of the loop.

-
Try to avoid the chained if commands, doing the same stuff – In both get_answer() and print_dice() you have if chains where the block kind of does the same. This is usually an indication that you could find a better way of handling it.

  • In get_answer() I opted for a double ternary, doing answer += 2 if i == 3 else 4 if i == 5 else 0.



-
In print_dice() I used the fact that a six sided die has only 5 distinct patterns, and then I used yet another double ternary to select from the different options.

Disclaimer: Even though this code is a lot shorter than yours, that doesn't mean it is nicer as such. But it does show an alternate way, and there are multiple ways to print die results. See here or here.

-
Use constants, not magic number – Instead of hiding a 5 and an 8 in the code, use constants at top of file like REQUIRED_STREAK_RUN = 8 and NUMBER_OF_DICE = 5. This allows you to change them easily (and you can still make the document reflect the change using str.format()

-
Use str.format() for string formatting - This is a very useful construct, which is well worth some time to look into. You could either use it without names (see end of play_once()), or you could name your variables

Code Snippets

"""Pythons Around the Rose

The name of the game is Pythons Around the Rose.
The name is important. I will roll {number_of_dice} dice,
and I will tell you how many pythons there are.

For example: 

| .   . |       | .   . | .     | .   . |
|   .   |   .   |       |   .   |   .   |
| .   . |       | .   . |     . | .   . |

Will result in 10 pythons. If you answer 
correctly a given {streak_run} number of times you
will be declared a "Pythentate of the Rose".
"""
import random
import sys

REQUIRED_STREAK_RUN = 8
NUMBER_OF_DICE = 5

def display_help():
    """Use the docstring of the module."""
    print(__doc__.format(number_of_dice = NUMBER_OF_DICE,
                         streak_run = REQUIRED_STREAK_RUN))

def play():
    """Play until declared a Pytentate..."""
    streak = 0
    while True:

        # My bug: streak += play_once(roll_multiple_dice(NUMBER_OF_DICE)):
        if play_once(roll_multiple_dice(NUMBER_OF_DICE)):
            streak += 1
        else:
            streak = 0

        if streak == REQUIRED_STREAK_RUN:
            print('You are now declared a "Pytentate of the Rose"!')
            break

    print("Thank you for playing!")

def roll_multiple_dice(number_of_dice):
    """Return a list of die rolls."""
    return [roll_die() for _ in range(number_of_dice)]


def roll_die():
    """Return a single die roll."""
    return random.randrange(6) + 1


def play_once(results):
    """Play a single round, and return True if correct guess."""
    print("How many pythons here?")
    print_dice(results)

    guess = get_guess_of_pythons()

    # Simple bail out option... :-)
    if guess < 0:
        sys.exit(0)

    answer = number_of_pythons(results)
    if guess == answer:
        print("Correct!")
        return True

    print("Incorrect. The answer is {}".format(answer))
    return False


def get_guess_of_pythons():
    """Get input of pythons from user."""
    while True:
        try:
            return int(input("> "))
        except:
            print("\nOops! That is not a number. Try again: ")


def number_of_pythons(dice):
    """Return correct number of pythons."""
    return sum(2 if i == 3 else
               4 if i == 5 else
               0 for i in dice)


EYES = ["       |",
        "   .   |",
        " .     |", 
        "     . |",
        " .   . |"]

def print_dice(dice):
    """Return a three-line string of all the dice."""
    rows = ["|", "|", "|"]
    for i in dice:
        rows[0] += EYES[0] if i == 1 else \
                   EYES[2] if i < 4 else \
                   EYES[4]

        rows[1] += EYES[4] if i == 6 else \
                   EYES[1] if i % 2 == 1 else \
                   EYES[0]

        rows[2] += EYES[0] if i == 1 else \
                   EYES[3] if i < 4 else \
                   EYES[4]

    print('\n'.join(rows))


def main():
    display_help()
    play()

if __name__ == '__main__':
    main()

Context

StackExchange Code Review Q#114227, answer score: 22

Revisions (0)

No revisions yet.