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

n number of x on the y

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

Problem

Everyone knows "99 bottles of beer on the wall".

Mat's Mug made a comment about mugs on the wall in The 2nd Monitor and I realized I never wrote a "beer on the wall" program. But that seemed way too easy, so I made it a "n number of x on the y" instead. It also seemed like a good excuse to use templates in Python, which I hadn't done before.

import sys

n = int(sys.argv[1])
x = sys.argv[2]
y = sys.argv[3]

template = '''\
%i %s on the %s
%i %s
Take one down, pass it around
%i %s on the %s
'''

for n in range(n, 0, -1):
    print template % (n, x, y, n, x, n-1, x, y)


Usage:

python script.py 99 "mugs of beer" wall


For obvious reasons there's no way (that I know of) to remove multiples of x if n = 1. Sure, it's not hard to turn mugs of beer into mug of beer, but it's generic. One can input anything and I can't catch all methods of pluralization.

I prefer readability over 'clever code' and my naming is probably not too good.

Solution

Your code is simple and just works. For that reason, you can be happy and there is not much comment to give. However, being very picky just for the sake of learning, one could say various things.

print statements

You are using print without parenthesis. This is the one and only reason why your code wouldn't work on Python 3. Just to be future (well, actually Python 3 is not the future, it is the present) proof, you might as well add the 2 parenthesis.

String formatting

Instead of using the % format, you could use the format method. You'll find more information online. Also using named place holder makes things easier to read and avoid duplicated values.

You'd get something like :

template = '''\
{idx} {x} on the {y} 
{idx} {x}
Take one down, pass it around
{prev} {x} on the {y}
'''

for n in range(n, 0, -1):
    print(template.format(idx=n, prev=n-1, x=x, y=y))


Error handling

You should probably check the number of arguments provided and tell the user if anything is wrong.

Variable names

Even though n is acceptable, x and y do not convey much information, a better name can probably be found.

Code Snippets

template = '''\
{idx} {x} on the {y} 
{idx} {x}
Take one down, pass it around
{prev} {x} on the {y}
'''

for n in range(n, 0, -1):
    print(template.format(idx=n, prev=n-1, x=x, y=y))

Context

StackExchange Code Review Q#101389, answer score: 13

Revisions (0)

No revisions yet.