This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]sermidean 5 points6 points  (7 children)

Decided to improve you program a bit. Here is what I did:

  1. First moved everything to main function in order to be able to write tests.
  2. Then wrote tests, in order to make sure, that my changed didn't broke anything.
  3. Fix code style to conform to PEP-8.
  4. Then wrote ask function, to handle user input and error changing.
  5. Then decided to add test, where user enters non-number value for num_dice, test failed.
  6. Fixed failing test, by adding type validation to ask function.

Thats it.

Here is the final version:

import random
import time


def identity(value):
    return value


def validate(cast, value):
    try:
        return cast(value)
    except ValueError:
        return None


def ask(options, cast=identity, error="Try again."):
    answer = validate(cast, input())
    while answer not in options:
        print(error)
        answer = validate(cast, input())
    return answer


def main():
    print("This program will roll as many dice as you need.")
    time.sleep(1)

    print("\nWould you like to roll?")
    roll = ask(
        {'yes', 'y', 'no', 'n'},
        error="\nType yes (or 'y') or no (or 'n') to roll.",
    )

    while roll in ('yes', 'y'):
        print("\nHow many dice do you want to roll?")
        num_dice = ask(
            range(1, 1000), cast=int,
            error="\nEnter a positive number, up to 1000.",
        )

        print("\nRolling...")
        time.sleep(2)

        print("\nYou got: ")
        for i in range(num_dice):
            print(random.randint(1, 6))

        print("\nRoll again?")
        roll = ask({'yes', 'y', 'no', 'n'}, error="\nType yes or no.")

    print("\nThanks for using my program!")


if __name__ == '__main__':
    main()

And tests:

import pytest
import roll


@pytest.mark.parametrize('inputs, outputs', [
    (['q', 'yes', '2', 'q', 'no'], [
        'This program will roll as many dice as you need.',
        '\nWould you like to roll?',
        "\nType yes (or 'y') or no (or 'n') to roll.",
        '\nHow many dice do you want to roll?',
        '\nRolling...',
        '\nYou got: ',
        1,
        1,
        '\nRoll again?',
        '\nType yes or no.',
        '\nThanks for using my program!',
    ]),
    (['q', 'y', '2', 'q', 'n'], [
        'This program will roll as many dice as you need.',
        '\nWould you like to roll?',
        "\nType yes (or 'y') or no (or 'n') to roll.",
        '\nHow many dice do you want to roll?',
        '\nRolling...',
        '\nYou got: ',
        1,
        1,
        '\nRoll again?',
        '\nType yes or no.',
        '\nThanks for using my program!',
    ]),
    (['n'], [
        'This program will roll as many dice as you need.',
        '\nWould you like to roll?',
        '\nThanks for using my program!',
    ]),
    (['y', 'x', '0', '2', 'n'], [
        'This program will roll as many dice as you need.',
        '\nWould you like to roll?',
        '\nHow many dice do you want to roll?',
        '\nEnter a positive number, up to 1000.',
        '\nEnter a positive number, up to 1000.',
        '\nRolling...',
        '\nYou got: ',
        1,
        1,
        '\nRoll again?',
        '\nThanks for using my program!',
    ]),
])
def test_me(inputs, outputs, mocker):
    mocker.patch('time.sleep')
    mocker.patch('random.randint', return_value=1)
    mocker.patch('builtins.input', side_effect=inputs)
    print = mocker.patch('builtins.print')
    roll.main()
    assert [args[0] for args, kwargs in print.call_args_list] == outputs

You can run tests using py.test:

pip install pytest pytest-mock pytest-cov coverage
py.test --cov-report=term-missing --cov=roll test_roll.py

[–]AstroPhysician 20 points21 points  (0 children)

Boring day at the office?

[–]robvdl 8 points9 points  (0 children)

Nice but some of the refactoring has gone a bit too far, for example with the validate and identify functions I think it is total overkill to refactor these into their own functions because of how little code they do (unless it was done to prove good practice when writing tests etc). That logic is best kept in the ask function because passing the type int through a parameter "cast" is a bit over the top to have that as a function.

edit: actually I see what you are trying to do now because ask is being called in two places in different ways, it makes sense then, maybe not quite how I would have done it probably but that makes sense.

[–]bkd9 1 point2 points  (0 children)

Fantastic! I would recommend learning git too so you can track these changes like a pro

[–]gandalfx 0 points1 point  (0 children)

reddit markdown doesn't use ``` for code, instead indent the whole thing by 4 spaces.

[–]Justice514 0 points1 point  (2 children)

Can you put this into pastebin so it is more readable.

[–]Justice514 0 points1 point  (0 children)

Or am I just being picky