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

Beginner FizzBuzz in Python

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

Problem

This is my first attempt at solving Fizzbuzz. I've always been pretty weak at algorithms, so I'm trying to improve. Specific feedback on performance would be appreciated.

def fizzBuzz(fizzNumber, buzzNumber):
  i = 0
  for i in range(1, 101):
      fizzBuzzValues = [i%fizzNumber, i%buzzNumber, i%(fizzNumber * buzzNumber)]
      if(0 in fizzBuzzValues):
          if(fizzBuzzValues[2] == 0):
              print("Fizzbuzz")
          elif(fizzBuzzValues[0] == 0):
              print("Fizz")
          elif(fizzBuzzValues[1] == 0):
              print("Buzz")
          else:
              break
      else:
          print(i)

fizzBuzz(3, 5)

Solution

Specific feedback on performance would be appreciated.

There are some things to improve performance-wise:

  • on every single iteration, you are calculating all the 3 remainders, even though you need a single one (in the best case)



  • the 0 in fizzBuzzValues check is not really needed (and the "in list" is O(n) in general, even though n=3 in this case, but you are doing that in a loop) - you can compare each of the remainders one by one



  • if you can calculate something before the loop - always do it - the fizzNumber * buzzNumber can be defined before the loop



Modified code:

def fizz_buzz_new(fizz_number, buzz_number):
    fizz_buzz_number = fizz_number * buzz_number
    for i in range(1, 101):
        if i % fizz_buzz_number == 0:
            print("Fizzbuzz")
        elif i % fizz_number == 0:
            print("Fizz")
        elif i % buzz_number == 0:
            print("Buzz")
        else:
            print(i)


Some timeit tests (replaced print() with yield to avoid seeing impact of printing to stdout):

$ ipython3 -i test.py 

In [1]: %timeit for x in range(100): list(fizzBuzz(3, 5))
100 loops, best of 3: 3.52 ms per loop

In [2]: %timeit for x in range(100): list(fizz_buzz_new(3, 5))
100 loops, best of 3: 1.98 ms per loop

Code Snippets

def fizz_buzz_new(fizz_number, buzz_number):
    fizz_buzz_number = fizz_number * buzz_number
    for i in range(1, 101):
        if i % fizz_buzz_number == 0:
            print("Fizzbuzz")
        elif i % fizz_number == 0:
            print("Fizz")
        elif i % buzz_number == 0:
            print("Buzz")
        else:
            print(i)
$ ipython3 -i test.py 

In [1]: %timeit for x in range(100): list(fizzBuzz(3, 5))
100 loops, best of 3: 3.52 ms per loop

In [2]: %timeit for x in range(100): list(fizz_buzz_new(3, 5))
100 loops, best of 3: 1.98 ms per loop

Context

StackExchange Code Review Q#153730, answer score: 7

Revisions (0)

No revisions yet.