Your code is a straight-line solution to a straight-line problem. So congratulations! You are off to a much better start than you might feel.
Here are some obvious issues:
Why did you separate your docblock from your code? And why is the rationale not included in the docblock?
One of the ways to become a better programmer is to try to show empathy to "future you." You can do this by including more information than you might think is necessary about what you are doing and why you are doing it.
In this case, if you have written a program with these rules included, the by all means include the challenge itself in the program, as well as a link to the URL of the challenge, and the circumstances under which you are taking the challenge:
""" rule-of-cups.py
Dave K. and I were drinking last night (17 Sep 2019) and he bet me a round
of beers that I couldn't get more than +5 points on CodeReview for posting
the solution to this question.
DESCRIPTION:
(Inspired by Chandler's GameOfCups with Joey in "Friends".)
Program gets 5-digit zipcode from user. (Assume user won't make a mistake, and
will enter exactly 5 digits). Program awards points based on a series of
rules, and reports the total points earned at the end. The 8 rules are
embedded as comments in the code. For each rule, besides adding points (or
not) to the
total, rule displays "Rule _ got _ points, so total is now _" (It prints this
even if rule added 0 points to total).
RULES
+5 when first and last digit match
+6 when second digit is twice the first AND third digit is greater than
second or fourth digit
+7 if any 7 is in the zipcode
+8 when there's no "13" in MIDDLE the zipcode
+9 when all three middle digits match
+10 when third and fourth digits match
+11 when zipcode is palindrome (12121 == 12121, while 12345 != 54321)
"""
Why are you not using functions?
Yes, it's true that the individual steps here are small and simple. That's no reason not to put them into functions that express what they do. When you add that little bit of abstraction, it makes the program easier to understand. When you add a docstring to
your function it makes things even easier, and gives you a place to help FutureYou understand what you were doing or not doing.
def get_zipcode() -> str:
""" Get a zip code from the user.
A zip code is \d{5}. No validation, though, because of the spec.
"""
zipcode = input("Enter your zipcode: ")
return zipcode
def rule1(zipcode: str) -> int:
""" +5 when first and last digit match """
points = 5 if zipcode[0] == zipcode[-1] else 0
return points
def rule2(zipcode: str) -> int: ...
def rule3(zipcode: str) -> int: ...
def rule4(zipcode: str) -> int: ...
def rule5(zipcode: str) -> int: ...
def rule6(zipcode: str) -> int: ...
def rule7(zipcode: str) -> int: ...
Why are you not following Python's convention for main?
Simple rule: the only time you don't do this is inside python -c "<code>"
.
if __name__ == '__main__':
total_points = 0
zipcode = get_zipcode()
points = rule1(zipcode)
total_points += points
print(f"Rule 1 got {points} points, so total is now {total_points}")
...
points = rule7(zipcode)
total_points += points
print(f"Rule 7 got {points} points, so total is now {total_points}")
print(f"{zipcode} got {total_points} points!")
Why are you not making your code testable?
Especially when you are asking for suggestions on CodeReview, it's important to have
confidence that some suggested changes doesn't cause a failure. So encapsulate your
code in functions that you can call in some kind of test harness.
In this case, it means that your points calculation should also be a function:
if __name__ == '__main__':
zipcode = get_zipcode()
total_points = game_of_cups(zipcode)
print(f"{zipcode} got {total_points} points!")
Once you have that, you can write some test cases:
def game_of_cups(zipcode: str) -> int:
""" Compute total points according to the rules. Print the score
of each rule, with a running total.
+5 when first and last digit match
+6 when second digit is twice the first AND third digit is greater than
second or fourth digit
+7 if any 7 is in the zipcode
+8 when there's no "13" in MIDDLE the zipcode
+9 when all three middle digits match
+10 when third and fourth digits match
+11 when zipcode is palindrome (12121 == 12121, while 12345 != 54321)
>>> game_of_cups('12345') == 8
Rule 1 got 0 points, so total is now 0
Rule 2 got 0 points, so total is now 0
Rule 3 got 0 points, so total is now 0
Rule 4 got 8 points, so total is now 8
Rule 5 got 0 points, so total is now 8
Rule 6 got 0 points, so total is now 8
Rule 7 got 0 points, so total is now 8
True
>>> game_of_cups('12321')
Rule 1 got 5 points, so total is now 5
Rule 2 got 6 points, so total is now 11
Rule 3 got 0 points, so total is now 11
Rule 4 got 8 points, so total is now 19
Rule 5 got 0 points, so total is now 19
Rule 6 got 0 points, so total is now 19
Rule 7 got 11 points, so total is now 30
30
"""
total_points = 0
... etc ...
Then you can run python -m doctest game-of-cups.py
and see some discouraging news:
(so) aghast@laptop:~/Code/so$ python -m doctest game-of-cups.py
**********************************************************************
File "/home/aghast/Code/so/game-of-cups.py", line 92, in game-of-cups.game_of_cups
Failed example:
game_of_cups('12345')
Expected:
Rule 1 got 0 points, so total is now 0
Rule 2 got 0 points, so total is now 6
Rule 3 got 0 points, so total is now 6
Rule 4 got 8 points, so total is now 14
Rule 5 got 0 points, so total is now 14
Rule 6 got 0 points, so total is now 14
Rule 7 got 0 points, so total is now 14
14
Got:
Rule 1 got 0 points, so total is now 0
Rule 2 got 6 points, so total is now 0
Rule 3 got 0 points, so total is now 0
Rule 4 got 8 points, so total is now 8
Rule 5 got 0 points, so total is now 8
Rule 6 got 0 points, so total is now 8
Rule 7 got 0 points, so total is now 8
8
**********************************************************************
File "/home/aghast/Code/so/game-of-cups.py", line 101, in game-of-cups.game_of_cups
Failed example:
game_of_cups('12321')
Expected:
Rule 1 got 5 points, so total is now 5
Rule 2 got 6 points, so total is now 11
Rule 3 got 0 points, so total is now 11
Rule 4 got 8 points, so total is now 19
Rule 5 got 0 points, so total is now 19
Rule 6 got 0 points, so total is now 19
Rule 7 got 11 points, so total is now 30
30
Got:
Rule 1 got 5 points, so total is now 5
Rule 2 got 6 points, so total is now 11
Rule 3 got 0 points, so total is now 11
Rule 4 got 8 points, so total is now 19
Rule 5 got 0 points, so total is now 19
Rule 6 got 0 points, so total is now 19
Rule 7 got 0 points, so total is now 19
19
Which leads me to point out that:
If you have functions, you can insert doctests in lots of places.
Like this:
def rule2(zipcode: str) -> int:
""" +6 when second digit is twice the first AND third digit is greater
than second or fourth digit
>>> rule2('12345')
6
>>> rule2('13431')
0
>>> rule2('12321')
6
"""
points = 6 if (int(zipcode[1]) * 2) > int(zipcode[0]) and (int(zipcode[2]) > int(zipcode[1]) or int(zipcode[2]) > int(zipcode[3])) else 0
return points
And yeah, your code here is wrong. You compute d1 * 2 > d0
but the rule says "is twice", which means ==
. Also, the second clause could be interpreted two ways. You have interpreted it as d1 < d2 or d3 < d2
, but it's probably worth some emphasis or clarification or something.
Also here:
def rule7(zipcode: str) -> int:
""" +11 when zipcode is palindrome (12121 == 12121, while 12345 != 54321)
>>> rule7('12321')
11
"""
# points = 11 if zipcode == reversed(zipcode) else 0
points = 11 if zipcode == zipcode[::-1] else 0
return points
This fails because reversed()
returns a reversed
iterator object, not a string. Using the [::-1]
reversing idiom works, though.
Finally, some repetition!
Finally, you'll notice that while the structure of the rules is similar, the contents of the rule functions is all different. I'm going to ignore that, since this is a pretty simple script.
However, the high-level gap_of_cups
function has got 7 calls to rule-functions which look identical except for the rule number in two places.
points = rule1(zipcode)
total_points += points
print(f"Rule 1 got {points} points, so total is now {total_points}")
We can take advantage of the fact that in Python, functions are first-class
objects. Just put the functions in an iterable, and iterate over them.
However, there's the question of how to print "Rule 1". We could use enumerate
to keep an integer value, or we could actually use the f.__name__
attribute of function objects. Since it's shorter and easier, let's just use enumerate
:
for i, rule in enumerate([rule1, rule2, rule3, rule4, rule5, rule6, rule7]):
points = rule(zipcode)
total_points += points
print(f"Rule {i} got {points} points, so total is now {total_points}")