17
\$\begingroup\$

I'm developing an implementation of the FizzBuzz game as part of an edutainment game for a retro video game platform. In its manual, I want to mention the history of writing a bot for the FizzBuzz game as a programmer screening test since Jeff Atwood's blog post, including sample answers in a few popular programming languages. But I don't want to confuse readers with poorly written code.

So is there anything unclear or unpythonic about my approach to FizzBuzz?

# Each of several divisors has a name. For each number,
# print the names of all divisors that divide the number,
# if any.  Otherwise print the number itself.
divs = [
    (3, "fizz"),
    (5, "buzz"),
]
for n in range(1, 101):
    # Make a string containing names of divisors of n
    ndivs = ''.join(name for (divisor, name) in divs
                    if n % divisor == 0)
    # If the string is empty, print the number instead
    print(ndivs or n)
Improve this question
\$\endgroup\$
0
3
\$\begingroup\$

I think the naming used here is a little confusing for new coders.

The name div would be read as a divisor, and elements called divisor are then extracted from it.

Perhaps use something with more clear indication of what it is, like divsor_pairs or divsor_names

divsor_pairs = [
    (3, "fizz"),
    (5, "buzz"),
]
for index in range(1, 101):
    divsor_output = ''.join(name for (divisor, name) in divsor_pairs
                    if index % divisor == 0)
    print(divsor_output or index)
Improve this answer
\$\endgroup\$
10
\$\begingroup\$

Nicely done! The great thing about this implementation that it's easy to extend (for example if you want to add (7, "jazz"). I have only some minor nitpicks.

Tuples are immutable, as opposed to lists. Since the list of fizz-buzz-jazz definitions don't change during the run of the program, the divs list can be a tuple. Tuples are faster than lists.

divs = (
    (3, "fizz"),
    (5, "buzz"),
)

The parentheses are unnecessary around (divisor, name):

ndivs = ''.join(name for (divisor, name) in divs
                if n % divisor == 0)

This may be a matter of taste, but I'd find this line more readable on a single line:

ndivs = ''.join(name for divisor, name in divs if n % divisor == 0)
Improve this answer
\$\endgroup\$
7
  • 1
    \$\begingroup\$ I slightly object to using a tuple for divs because a) speed is irrelevant here and b) tuples are often used for inhomogenous data (position in tuple matters) while lists are often used for homogenous data. \$\endgroup\$ – Jasper Dec 15 '14 at 10:10
  • \$\begingroup\$ @Jasper speed is not the main concern, that's why I added that as the last point, more of an additional good thing. The more important is that it's immutable, which is perfect for the purpose. Preferring a better technique, even if it's not required, is a good practice to build good habits \$\endgroup\$ – janos Dec 15 '14 at 10:15
  • \$\begingroup\$ @Jasper Can you please elaborate a bit on b) point? \$\endgroup\$ – confused00 Dec 15 '14 at 15:37
  • 1
    \$\begingroup\$ stackoverflow.com/a/1708702/3456281 and the second comment. E.g. if you have a coordinate pair, a tuple should be used because the first and second element have a different meaning, and adding elements to this pair also makes no sense, because your coordinate lives in 2D or 3D (or any other fixed-D). \$\endgroup\$ – Jasper Dec 15 '14 at 16:02
  • 2
    \$\begingroup\$ @confused00 In other words, the conceptual difference is analogous to that between an array and a struct in C, except that a tuple is immutable. The semantics of Python list invite appending more divisor-name pairs, such as (7, "feed") to produce clickbait at 35. The only speed difference is at construction time anyway. \$\endgroup\$ – Damian Yerrick Dec 15 '14 at 17:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.