patternpythonMinor
FizzBuzz in Python using a class
Viewed 0 times
usingclasspythonfizzbuzz
Problem
I recently read the book clean code by Robert C. Martin and I'm trying to apply his style of writing methods to the FizzBuzz programming example.
So far I have two "public" methods called:
I'm happy with the last 3 methods that are clearly doing one thing. But the
So far I have two "public" methods called:
start and parseNumber, that respectively start the FizzBuzz program or parse a single number according to the FizzBuzz principle.class FizzBuzz(object):
def start(self, end_number):
return ",".join(self._parse_numbers(end_number))
def _parse_numbers(self, end_number):
number_list = []
for number in range(1, end_number+1):
number_list.append(self.parse_number(number))
return number_list
def parse_number(self, number):
if self._is_number_divisible_by_five_and_three(number):
return "FizzBuzz"
elif self._is_number_divisible_by_three(number):
return "Fizz"
elif self._is_number_divisible_by_five(number):
return "Buzz"
else:
return str(number)
def _is_number_divisible_by_five_and_three(self, number):
return number % 15 == 0
def _is_number_divisible_by_three(self, number):
return number % 3 == 0
def _is_number_divisible_by_five(self, number):
return number % 5 == 0I'm happy with the last 3 methods that are clearly doing one thing. But the
_parse_numbers and parse_number methods can be improved, I just don't know how to do that.Solution
First of all, you don't need to make a class for this. There's no real reason for
You could also add a docstring and maybe a comment, to indicate what the function does and returns as well as noting how
Start seems like a not very worthwhile function and not a great name if you're no longer making a class. But if you want to keep it a name like
Now, your
Now you can easily just call
FizzBuzz to be an object when all you have is a collection of functions anyway. Likewise, you have a lot of functions that don't really need to be functions. All your test functions could just be plain if statements and that'd make them more readable to Python. I see you've discovered the _ naming convention in Python, where names beginning with that are to be considered private. However you don't need to use these that often. Nothing's ever going to be truly private in Python without a lot of work, so instead allow people to access functions if they want to. Only mark functions private if there's actual problems with them attempting to use the function outside of a specific process.parse_number is good, though I'd replace the tests with plain tests instead of your functions.def parse_number(number):
if number % 15 == 0:
return "FizzBuzz"
elif number % 3 == 0:
return "Fizz"
elif number % 5 == 0:
return "Buzz"
else:
return str(number)You could also add a docstring and maybe a comment, to indicate what the function does and returns as well as noting how
number % 15 == 0 is the same as (number % 5 == 0) and (number % 3 == 0).def parse_number(number):
"""Returns a string Fizzbuzz representation of a number"""
# This is the same as (number % 5 == 0) and (number % 3 == 0)
if number % 15 == 0:Start seems like a not very worthwhile function and not a great name if you're no longer making a class. But if you want to keep it a name like
fizzbuzz_string would be better.Now, your
_parse_numbers isn't a great name because it's too similar to an existing function. There's also little reason to mark it as private. Instead this could be your main method, or at least call it fizzbuzz. You could also build it easier and faster with something called a list comprehension. A list comprehension is basically a for loop like expression that will be evaluated to create a list. Yours is very simple, you just need to run parse_number on each number in your range, so this is how you could write it: def parse_number(number):
"""Returns a string Fizzbuzz representation of a number"""
# This is the same as (number % 5 == 0) and (number % 3 == 0)
if number % 15 == 0:
return "FizzBuzz"
elif number % 3 == 0:
return "Fizz"
elif number % 5 == 0:
return "Buzz"
else:
return str(number)
def fizzbuzz(end_number):
"""Return a list of Fizzbuzz parsed numbers up to end_number."""
return [parse_number(number) for number in range(1, end_number+1)]
def fizzbuzz_string(end_number):
return ",".join(fizzbuzz(end_number))Now you can easily just call
print fizzbuzz_string(number) and get the full list without needing an object or the other extraneous functions.Code Snippets
def parse_number(number):
if number % 15 == 0:
return "FizzBuzz"
elif number % 3 == 0:
return "Fizz"
elif number % 5 == 0:
return "Buzz"
else:
return str(number)def parse_number(number):
"""Returns a string Fizzbuzz representation of a number"""
# This is the same as (number % 5 == 0) and (number % 3 == 0)
if number % 15 == 0:def parse_number(number):
"""Returns a string Fizzbuzz representation of a number"""
# This is the same as (number % 5 == 0) and (number % 3 == 0)
if number % 15 == 0:
return "FizzBuzz"
elif number % 3 == 0:
return "Fizz"
elif number % 5 == 0:
return "Buzz"
else:
return str(number)
def fizzbuzz(end_number):
"""Return a list of Fizzbuzz parsed numbers up to end_number."""
return [parse_number(number) for number in range(1, end_number+1)]
def fizzbuzz_string(end_number):
return ",".join(fizzbuzz(end_number))Context
StackExchange Code Review Q#109652, answer score: 7
Revisions (0)
No revisions yet.