26
\$\begingroup\$

I just recently started working on python (interesting language, actually), and, as I always do when I learn a new language (with the exception of BrainFuck, that's too hard), I write a FizzBuzz program.

I notice there is no error-checking, so my program would have problems with incorrect input, but please ignore that as I haven't gotten to that part yet.

Code:

def getString(num, fizz, buzz):
    if num % (fizz * buzz) == 0:
        return "FizzBuzz"
    elif num % fizz == 0:
        return "Fizz"
    elif num % buzz == 0:
        return "Buzz"
    else:
        return num

def fizzbuzz(maxNum, fizz, buzz):
    num = 1;
    while num <= maxNum:
        print(getString(num, fizz, buzz))
        num += 1

fizz = int(input("Enter the number to Fizz: "))
buzz = int(input("Enter the number to Buzz: "))
maxNum = int(input("Enter the maximum number: "))
fizzbuzz(maxNum, fizz, buzz)

The code will take a fizz, buzz, and a maxNum as input, and will output, all the fizzing and buzzing the code is doing.

Concerns:

  1. Does it follow Python conventions?
\$\endgroup\$
  • 10
    \$\begingroup\$ Note that your logic is actually wrong. getString(4, 2, 4) == "Fizz" when it should actually equal "FizzBuzz". This bug occurs when fizz and buzz are not coprime. \$\endgroup\$ – Bill Lynch Dec 15 '15 at 4:14
  • 3
    \$\begingroup\$ FizzBuzz in Brainfuck too hard? Madness. \$\endgroup\$ – Mast Dec 15 '15 at 12:03
  • 1
    \$\begingroup\$ Relevant answer on Code Golf. \$\endgroup\$ – Martin Ender Dec 15 '15 at 17:00
  • \$\begingroup\$ It never hurts to keep things up to par with pylint 's standards. Possibly pep8 's too. \$\endgroup\$ – ThorSummoner Dec 29 '15 at 10:38
  • \$\begingroup\$ Note: The bug described by @BillLynch can be avoided by using if num % fizz == 0 and num % buzz == 0: \$\endgroup\$ – Gloweye Oct 9 at 8:23
24
\$\begingroup\$
  • getString and maxNum should be get_string and max_num, by PEP 8 recommendation.
  • getString sometimes returns a number, and sometimes returns a string. This violates the expectation set up by the function name. Also, a function that is indecisive about its return type is harder to work with.
  • num = 1; ← No semicolons in Python, please.
  • The fizzbuzz loop is written idiomatically as for num in range(1, maxNum + 1): …
\$\endgroup\$
  • 1
    \$\begingroup\$ Wow, I didn't notice the semicolon, but it runs fine with is there. Is there a reason for that? \$\endgroup\$ – TheCoffeeCup Dec 15 '15 at 1:05
  • 2
    \$\begingroup\$ Python prefers using proper code layout for separating statements. Putting multiple statements on the same line, separated by semicolons, is discouraged. The newline suffices to end a statement (unless it's preceded by a backslash; or is within a unmatched (, {, or [; or it's within a triple-quoted string). So, there is no need to clutter your code with excess punctuation. \$\endgroup\$ – 200_success Dec 15 '15 at 1:10
  • \$\begingroup\$ @TheCoffeeCup In case it's still unclear. Python can use semicolons to signal newlines, but you don't need to (unless you're trying to put multiple statements on one line, which is discouraged anyway). \$\endgroup\$ – SuperBiasedMan Dec 17 '15 at 11:31
  • \$\begingroup\$ wouldn't xrange be more idiomatic? \$\endgroup\$ – chicks Dec 21 '15 at 17:32
  • \$\begingroup\$ @chicks The question is tagged as python-3.x. There is no xrange(). Even if it were Python 2, I would just use range() for compatibility if the range is small and performance is not critical. \$\endgroup\$ – 200_success Dec 22 '15 at 5:02
15
\$\begingroup\$

In addition to other answers here is a common way to avoid having too much code on top level:

def main():
    fizz = int(input("Enter the number to Fizz: "))
    buzz = int(input("Enter the number to Buzz: "))
    maxNum = int(input("Enter the maximum number: "))
    fizzbuzz(maxNum, fizz, buzz)

if __name__ == '__main__':
    main()

This patterns allows for your script to be called from the command line, and it also allows for use as an module, so that you reuse your fizzbuzz() function.

And whilst at it, even though this is a rather simple functions, please look into using docstrings to document your function and potentially your module.

\$\endgroup\$
3
\$\begingroup\$

In addition to previous answers:

It is considered more Pythonic to check if not num % fizz*buzz than if num % fizz*buzz == 0

Also you don't need to use elif if there is a return statement inside if, so I'd rewrite your get_string function like this:

def get_string(num, fizz, buzz)
    if not num % fizz*buzz:
        return "FizzBuzz"
    if not num % fizz:
        return "Fizz"
    if not num % buzz:
        return "Buzz"
    return str(n)
\$\endgroup\$
  • 1
    \$\begingroup\$ In general I agree that using not something is better than something != 0 or something is not None... When used in conjunction with the modulo operator, I'm a little more sceptic. It kinda reads if not num then take the modulo of the not num afterwards. \$\endgroup\$ – holroy Dec 16 '15 at 21:33
  • \$\begingroup\$ Will parentheses help to clarify that? \$\endgroup\$ – StorymasterQ Dec 18 '15 at 0:54
2
\$\begingroup\$

I notice there is no error-checking, so my program would have problems with incorrect input

Lack of error checking, eg duck typing is generally approved of and preferred in python. Surely there are good cases to use input checking, but generally duck-typing is preferred.

I suspect this preference in python is mostly due to all the "primitives" actually being first class objects: int, str, bool. They all have values, methods, and identities.

Besides, type checking often just gets in the way of your users. Say I use a integer-like class that is tied to an active record or has other features that make it better to work with in my application; a type check is just a barrier to entry that maybe I don't care to hoop-jump for. I use my integer class in place of integers everywhere in my app just fine, without type checking; your code may/would have optimistically run with whatever given object and worked just fine too. Functions that quit just because say an input object doesn't identify as an integer are just plain annoying.

Of course other subtle and clever advantages could be leveraged too. A caller could cleverly integrate with your function in rich ways, passing an impersonated integer-compatible object with valuable performance/analytics tracking and/or whatever else might be needed for a proper unit test, or an object that just knows that when certain maths is applied, that it should update a progress bar. Whatever odd edge case matters, Python so often has an "in". The "we're all consenting adults" dogma that doesn't shield any part of the runtime from good (or bad) ideas, lets the analytics engineer harmlessly track the core components, and lets the performance engineer substitute in-place code with bootstrapping and wrappers that hold over until something can be re-architected.

If you are to pursue type checking I would at most suggest catching the kind of operational arithmetic error that incompatible types would cause and raise instead a type error, or a specific-to-your-module exception that probably extends TypeError, when the input has produced an apparent failure condition.

The value in extending a module-specific exception is that it gives any caller a sure way to catch errors only from your module, without giving up valuable semantics or messages.

\$\endgroup\$

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.