Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Below are two solutions to the Fizz Buzz problem in Python. Which one of these is more "Pythonic" and why is it more "Pythonic" than the other?

Solution One:

fizzbuzz = ''

start = int(input("Start Value:"))
end = int(input("End Value:"))

for i in range(start,end+1):
    if i%3 == 0:
        fizzbuzz += "fizz"
    if i%5 == 0:
        fizzbuzz += "buzz"
    if i%3 != 0 and i%5 != 0:
        fizzbuzz += str(i)

    fizzbuzz += ' '

print(fizzbuzz)

Solution Two:

fizzbuzz = []

start = int(input("Start Value:"))
end = int(input("End Value:"))

for i in range(start,end+1):
    entry = ''
    if i%3 == 0:
        entry += "fizz"
    if i%5 == 0:
        entry += "buzz"
    if i%3 != 0 and i%5 != 0:
        entry = i

    fizzbuzz.append(entry)

for i in fizzbuzz:
    print(i)
share|improve this question

6 Answers

up vote 11 down vote accepted

As has already been pointed out, creating a list is preferable as it avoids the concatenation of large strings. However neither of your solutions is the most pythonic solution possible:

Whenever you find yourself appending to a list inside a for-loop, it's a good idea to consider whether you could use a list comprehension instead. List comprehensions aren't only more pythonic, they're also usually faster.

In this case the body of the loop is a bit big to fit into a list comprehension, but that's easily fixed by refactoring it into its own function, which is almost always a good idea software design-wise. So your code becomes:

def int_to_fizzbuzz(i):
    entry = ''
    if i%3 == 0:
        entry += "fizz"
    if i%5 == 0:
        entry += "buzz"
    if i%3 != 0 and i%5 != 0:
        entry = i
    return entry

fizzbuzz = [int_to_fizzbuzz(i) for i in range(start, end+1)]

However, while we're at it we could just put the whole fizzbuzz logic into a function as well. The function can take start and end as its argument and return the list. This way the IO-logic, living outside the function, is completely separated from the fizzbuzz logic - also almost always a good idea design-wise.

And once we did that, we can put the IO code into a if __name__ == "__main__": block. This way your code can be run either as a script on the command line, which will execute the IO code, or loaded as a library from another python file without executing the IO code. So if you should ever feel the need to write a GUI or web interface for fizzbuzz, you can just load your fizzbuzz function from the file without changing a thing. Reusability for the win!

def fizzbuzz(start, end):
    def int_to_fizzbuzz(i):
        entry = ''
        if i%3 == 0:
            entry += "fizz"
        if i%5 == 0:
            entry += "buzz"
        if i%3 != 0 and i%5 != 0:
            entry = i
        return entry

    return [int_to_fizzbuzz(i) for i in range(start, end+1)]

if __name__ == "__main__":
    start = int(input("Start Value:"))
    end = int(input("End Value:"))
    for i in fizzbuzz(start, end):
        print(i)

(Note that I've made int_to_fizzbuzz an inner function here, as there's no reason you'd want to call it outside of the fizzbuzz function.)

share|improve this answer
You could also take start end as command line arguments (as opposed to user input) so any future GUI can call the command line version in its present state without having to refactor any code. You'll probably need to add a --help argument so users have a way to look up what arguments are available. – Evan Plaice Feb 16 '11 at 0:10
I think those last two lines could better be print '\n'.join(fizzbuzz(start, end)). Also, refactored like this, the third if can be written if not entry:. Also, writing a bunch of Java may have made me hypersensitive, but entry = str(i) would make the list be of consistent type. – pjz Apr 28 '12 at 2:22

From my brief experience with Python, I would say the second is more Pythonic as it takes advantage of the Python lists and the first is just appending to strings which causes the output to be a wee bit ugly and clumped together. Although you could eliminate an entire extra iteration by appending to a temporary string within the first for loop, printing at the end of the loop and resetting the value so that the values don't need to be stored for more than one iteration.

share|improve this answer

this does not respond to your answer but i think respond to "be pythonc" issue

i have read a good solution with decorator, and i think it is a pythonic way to achive fizzbuzz.

something like:

@fizzbuzzness( (3, "fizz"), (5, "buzz") )
def f(n): return n

also generators is a good pythonic way to get a list of numbers..

share|improve this answer

I recently did "fizz buzz zing":

from bisect import bisect_left

__author__ = 'Robert'

def two():
    while True:
        yield ""
        yield "Fizz"

def three():
    while True:
        yield ""
        yield ""
        yield "Buzz"

def five():
    exclamation_mark = False
    while True:
        yield ""
        yield ""
        yield ""
        yield ""
        yield "Zing!" if exclamation_mark else "Zing"

class BonusPoints(list):
    def __getitem__(self, x):
        return x - (x/2 + x/3 + x/5 - x/6 - x/10 - x/15 + x/30) #find min x s.t. f(x) = number

    def find(self, number=1000):
        #by default it finds the 1000th number that does not have any "fizz", "buzz" or "zing"
        return bisect_left(self, number, hi=30*number)

def get_lines():
    n = input()
    assert(isinstance(n, int))
    for values in zip(range(1, n + 1), two(), three(), five()):
        line = "%s: %s%s%s" % values
        yield line.strip()
    yield "Bonus - 1000th = %s" % BonusPoints().find()

with open('output.txt', 'w') as out_file:
    out_file.writelines("\n".join(get_lines()))
share|improve this answer

There is no difference in how "Pythonic" those solutions are. Both are perfectly acceptable. If the fizzbuzz string gets very long, using a list is preferable, as you don't have to make a copy of the string in every iteration, but that's a very minor issue.

share|improve this answer

Most definitely solution two. However, you could further improve that solution by dumping the string concatenation altogether. Remember, string concatenation is expensive. Because strings are immutable, every time you concatenate, a new string is created. While the garbage collector can pick up the trash later, you're still having to go through the expense of copying a string.

Instead, I'd recommend making a format string and then inserting the items in via string formatting, then appending them to your fizzbuzz list.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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