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

MontyPython (Fizzbuzz)

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

Problem

This is an iterative review. The previous iteration can be found here.

I've now been programming in python for all of 6 hours.

This is probably my last iteration of FizzBuzz.

Thoughts?

FizzBuzz.py

from utilities import ask_something
from utilities import ask_yes_no

def print_fizz_buzz_output(start_num, end_num, pair_list):
    for num in range(start_num, end_num + 1):
        divisor_text = [text for divisor, text in pair_list if num % divisor == 0]
        if divisor_text:
            print(''.join(divisor_text))
        else:
            print(num)

def ask_divisor_text_pairs():
    loop = True
    while loop:
        divisor = ask_something('Divisor? ', int,
                                'Invalid input for divisor. Divisor must be a whole number. Please try again.')
        text = ask_something('Text? ', str)
        yield (divisor, text)

        loop = (True if 'y' == ask_yes_no('Input another Divisor (y/n)? ') else False)

def ask_iteration_range() -> tuple:
    start_num = ask_something('Start Number? ', int)
    while True:
        end_num = ask_something('End Number? ', int)
        if end_num >= start_num:
            break
        else:
            print('Invalid input for end number. Must be a whole number greater than or equal to the start number.'
                  ' Please try again.')
    return start_num, end_num

if __name__ == '__main__':
    print_fizz_buzz_output(*ask_iteration_range(), list(ask_divisor_text_pairs()))


utilities.py

```
def ask_something(prompt: str, convert, error_message='Invalid input.'):
while True:
value = input(prompt)
try:
return convert(value)
except ValueError:
print(error_message)

def ask_yes_no(prompt: str, allow_upper_case=True) -> str:
# returns 'y' or 'n'
while True:
value = ask_something(prompt, str)
if value == 'y' or (value == 'Y' and allow_upper_case):
return 'y'
elif value == 'n' or (value == 'N

Solution

The code is good!

I however do have a couple of niggles:

-
You should use commas to import multiple functions from a module.
This changes the imports at the top to a single import:

from utilities import ask_something, ask_yes_no


-
== results in a boolean, no need to make on yourself.

>>> ('y' == 'y', 'y' == 'n')
(True, False)


And so you can simplify the reassignment of the loop flag.

-
If I already can't read half of a string, you may as well keep it all on one line.
Whilst PEP8 says that we should limit the line length to 79 characters,
formatting large chunks of words within this requirement become an eye saw to me.
And so I'd just leave them on one line.

I also don't really like the verbose messages:


Invalid input for divisor. Divisor must be a whole number. Please try again.

The message "The input must be a whole number." is enough.
Alternately you could just re-ask the question,
which implies there's been an error.

Code Snippets

from utilities import ask_something, ask_yes_no
>>> ('y' == 'y', 'y' == 'n')
(True, False)

Context

StackExchange Code Review Q#143423, answer score: 4

Revisions (0)

No revisions yet.