class fizzbuzz():

Question

This is now an iterative review. The next iteration can be found here.

---

Yep. Fizzbuzz in Python.

I've been coding in Python for all of 90 minutes.

Thoughts?

---

Program Flow

Take a start_num and an end_num as user input

Ask for divisor/value pairs until user inputs a response that is not y

Iterate over the specified range, appending text in the order it was input where appropriate.

---

FizzBuzz.py

class divisor_text_pair():

    def __init__(self,divisor,text):
        self.divisor=divisor
        self.text=text

class fizzbuzz():

    start_num = int(input('Start Number? '))
    end_num =int(input('End Number? '))

    pair_list = []

    response = 'y'
    while response == 'y':

        divisor = int(input('Divisor? '))
        text = input('Text? ')
        pair = divisor_text_pair(divisor, text)
        pair_list.append(pair)
        response = input('Input Another Divisor (y/n)? ')

    def print_numbers_and_texts(start_num, end_num, pair_list):

        for num in range(start_num, end_num + 1):

            out_text = str(num)

            for pair in pair_list:

                if num % pair.divisor == 0:
                    out_text += ' '
                    out_text += pair.text

            print(out_text)

    print_numbers_and_texts(start_num, end_num, pair_list)

fizzbuzz()

---

Example Input/Output:

Start Number? 1
End Number? 10
Divisor? 2
Text? Fizz
Input Another Divisor (y/n)? y
Divisor? 3
Text? Buzz
Input Another Divisor (y/n)? n
1
2 Fizz
3 Buzz
4 Fizz
5
6 Fizz Buzz
7
8 Fizz
9 Buzz
10 Fizz

Answer 1

You do not need a class

The only method you have is static (it does not take self as argument) so you do not need a class.

The Pair class is also unnecessary because of tuple unpacking.

Please avoid over-complicating Fizz-Buzz.

Input handling in a separate function

Instead of top-level into the class.

Final solution

def print_numbers_and_texts(start_num, end_num, pair_list):
    for num in range(start_num, end_num + 1):
        print(num, end='')
        for divisor, text in pair_list:
            if num % divisor == 0:
                print(' ' + text)

def ask_pairs():
    while True:
        yield (int(input("Divisor? ")), input("Text? "))
        if input('Input Another Divisor (y/n)?') == 'n':
            break

print_numbers_and_texts(
    int(input('Start Number? ')),
    int(input('End Number? ')),
    list(ask_pairs())
)

Answer 2

Apart from what Caridorc said in his answer:

Some PEP8 related advices that it's good to follow while writing python code:

• class names should follow CamelCase convention
• around operators it's ok to have one space
• after , there should also be a space
• between classes / methods you should have 2 lines instead of just one

Use if __name__ == '__main__':

One of the reasons for doing this is that sometimes you write a module (a .py file) where it can be executed directly. Alternatively, it can also be imported and used in another module. By doing the main check, you can have that code only execute when you want to run the module as a program and not have it execute when someone just wants to import your module and call your functions themselves.

More, I agree with the above answer which states that usage of classes in this program doesn't bring any advantages. Just a headache. Don't try to over-complicate small programs by writing them with classes. There's an awesome thread out there which is called: Object Oriented Programming is an expensive disaster which must end.

While I don't agree with everything written in there, you should read it because it contains some useful infos.

Using what Caridorc proposed, I'd add this:

def print_numbers_and_texts(start_num, end_num, pair_list):
    for num in range(start_num, end_num + 1):
        print num
        for divisor, text in pair_list:
            if num % divisor == 0:
                print ' ' + text


def ask_pairs():
    while True:
        yield (int(input("Divisor? ")), input("Text? "))
        if input('Input Another Divisor (y/n)?') == 'n':
            break


def main():
    print_numbers_and_texts(
        int(input('Start Number? ')),
        int(input('End Number? ')),
        list(ask_pairs())
    )

if __name__ == '__main__':
    main()

Answer 3

Handle invalid input

I found an issue with the way you handle some user input:

Start Number?  hello
Traceback (most recent call last):
  File "python", line 7, in <module>
  File "python", line 9, in fizzbuzz
ValueError: invalid literal for int() with base 10: 'hello'

Which is a consequence of this conversion to int without checking that it actually can be converted to int:

int(input('Start number? ')) # and others like it

You could use a while loop along with try...except to continue asking for input until it is usable:

input_is_valid = False
while not input_is_valid:
    start_num = input('Start Number? ')
    end_num =input('End Number? ')
    try:
        start_num = int(start_num)
        end_num = int(end_num)
        input_is_valid = True
    except ValueError:
        print('Invalid number input for start "{0}" or end "{1}".'.format(start_num, end_num))

It may also be worthwhile to consider adding another check to make sure the start number is less than the ending number to prevent confusing behavior if that were not the case.

---

Another user input related concern is this:

while response == 'y':

Albeit very minor, it treats capital Y as not yes. Simple fix of course:

while response.lower() == 'y':

---

Your class divisor_text_pair() (by the way, class names in Python should follow TitleCase) has no functionality beyond holding two values. Using a native namedtuple type instead would simplify the code while still allowing you to retrieve the values with dot notation x.divisor and x.text:

from collections import namedtuple

DivisorTextPair = namedtuple('DivisorTextPair', 'divisor text')

class FizzBuzz(): #...

The syntax to use a namedtuple is like that of a class:

pair = DivisorTextPair(divisor, text)

---

Class approach

While you class fizzbuzz() approach can be and has been critiqued, I would like to approach it from the angle actually using your class approach, but in a better way.

Your main class is really more of a procedure, as it does not have a constructor, and a lot of the logic is directly in the body of the class, rather than in methods.

P.S.: Note that I will be using Python 3.5 type hints in the method/function signatures, which I recommend to use.

---

In my refactor of your class (runnable demo on repl.it), the first section would be the constructor and what happens once __init__ is called:

class FizzBuzz():
    def __init__(self):
        self.start_fizzbuzz()

Along with a method to control the general flow:

    def start_fizzbuzz(self) -> None:
        """Controls the overall logic of a FizzBuzz program."""
        start_num, end_num = self.get_start_end_numbers()
        pair_list = self.get_divisor_text_pairs()
        self.print_numbers_and_texts(start_num, end_num, pair_list)

Note the use of Iterable Unpacking (often referred to as "tuple unpacking") in start_num, end_num = self.get_start_end_numbers() (that method will return a tuple, read on). The "long" way of writing the same thing without using tuple unpacking would be like this:

start_end_nums = self.get_start_end_numbers()
start_num = start_end_nums[0]
end_num = start_end_nums[1]

---

From there, we are getting the start and end numbers from a method where I moved the user input logic to:

def get_start_end_numbers(self) -> tuple:
    input_is_valid = False
    while not input_is_valid:
        start_num = input('Start Number? ')
        end_num =input('End Number? ')
        try:
            start_num = int(start_num)
            end_num = int(end_num)
            input_is_valid = True
        except ValueError:
            print('Invalid number input for start "{0}" or end "{1}".'.format(start_num, end_num))
    return start_num, end_num

The get_divisor_text_pairs() method is what you'd expect it to be, with above applied:

def get_divisor_text_pairs(self) -> list:
    pair_list = []
    response = 'y'
    while response.lower() == 'y':
        input_is_valid = False
        while not input_is_valid:
            divisor = input('Divisor? ')
            try:
                divisor = int(divisor)
                input_is_valid = True
            except ValueError:
                print('"{0}" is not a valid number.'.format(divisor))
        text = input('Text? ')
        pair = DivisorTextPair(divisor, text)
        pair_list.append(pair)
        response = input('Input Another Divisor (y/n)? ')
    return pair_list

print_numbers_and_texts() method is essentially unchanged (except for the type hints in the signature)