0
\$\begingroup\$

The fizzbuzz challenge presents a lot of interesting questions for 'intermediate' programmers like myself. I have found that I don't actually know where to draw the line with, complex (hard to read but concise/efficient) code and simple (easy to read but not concise) code. Additionally, I actually do not know if my 'more efficient' code, actually performs better, or is just an expectation that complexity=efficiency. I am sure it doesn't. I made the example below after pondering this for a while and running a few tests, it is a solution using a lambda function inside the list comp. Sort of functional programming but without meaning...

output = ["fizzbuzz", "buzz", "fizz"]
iterList = lambda x: [x%i for i in [15, 5, 3]]
fizzBuzz = [output[iterList(x).index(0)] if 0 in iterList(x) else x for x in range(1, 101)]
print(fizzBuzz)

The main concern I have is in regard to computational efficiency. While this code is concise, reasonably complex and works. Would it actually perform any faster than a simple 5-to-10 line if, else function? Is there any point in designating the lambda function to a variable? Why would anyone sacrifice readability for complexity if there is no incentive? I have so many questions, but I guess thats the curve with programming; you think you're so good until you realise you're so bad.

\$\endgroup\$
3
  • 2
    \$\begingroup\$ I see a lot of "is this faster?" and not a lot of "I tested this to see if it's faster". You should be executing comparative performance tests if that's what you care about. \$\endgroup\$
    – Reinderien
    Apr 23, 2022 at 15:16
  • \$\begingroup\$ @Reinderien oh I did do performance tests, the solution above was marginally faster than a simple if, else conditional function. I was asking about performance in more of a general sense, rather than this specific example of fizzbuzz. I know how much these questions are hated though, and I respect your criticism as the question was definitely too open. \$\endgroup\$
    – Dat Boi
    Apr 23, 2022 at 16:13
  • \$\begingroup\$ It's not a hated question at all, but certainly part of developing your skills is reaching for the right kind of test early in the process. \$\endgroup\$
    – Reinderien
    Apr 23, 2022 at 18:35

1 Answer 1

1
\$\begingroup\$

I don't want to post a full review but multi-line code should not be posted into comments.

Your code is not as efficient as you think.

lambda x: [x%i for i in [15, 5, 3]]

The above line is very inefficient, you correctly identified the precedence of the moduli, but you did not use it to short circuit the executions.

For example, 30 is a multiple of 15 and it should generate "FizzBuzz", here we only need to check whether it is a multiple of 15, because it is a multiple of 15 we don't need and shouldn't check whether it is a multiple of 5 or 3, your code performs two more redundant checks that only worsens the performance.

25 is a multiple of 5, we first check whether it is a multiple of 15, it is not, we then check whether it is a multiple of 5, it is, so log "Buzz", done, there is absolutely no reason to check if it is a multiple of 3.

So you should write it like this:

special = {15: 'FizzBuzz', 5: 'Buzz', 3: 'Fizz'}
for i in (15, 5, 3):
    if not number % i:
        var = special[i]
        break

break is a keyword that stops the loop, so it won't perform consecutive checks.

special is a dict, if you don't know what a dictionary is, look it up. It is much better than your double list indexing.

for loops also have an else clause, which executes after the loop is completed normally.

So if the number is not a multiple of any of the three, to get the number itself:

special = {15: 'FizzBuzz', 5: 'Buzz', 3: 'Fizz'}
for i in (15, 5, 3):
    if not number % i:
        var = special[i]
        break
else:
    var = number

We normally wrap code we want to use repetitively inside functions, and use return to return the output back to the caller, return automatically stops the function so anything after it will not be executed.

The function:

special = {15: 'FizzBuzz', 5: 'Buzz', 3: 'Fizz'}
def fizzbuzz(number):
    for i in (15, 5, 3):
        if not number % i:
            return special[i]
    return number

The function is much better than your three lines of code.

To print first 100 fizzbuzz values:

special = {15: 'FizzBuzz', 5: 'Buzz', 3: 'Fizz'}
def fizzbuzz(number):
    for i in (15, 5, 3):
        if not number % i:
            return special[i]
    return number
for i in range(1, 101):
    print(fizzbuzz(i))

Update


If you only care about performance, here is a one-liner that uses generator expressions and next and I won't explain any of these two.

SPECIAL = {15: 'FizzBuzz', 5: 'Buzz', 3: 'Fizz'}

def fizzbuzz(number):
    return next((label for x, label in SPECIAL.items() if not number % x), number)

Ultimate code golf:

a='Fizz';b='Buzz';S={15:a+b,5:b,3:a};f=lambda n:next((l for c,l in S.items()if not n%c),n)

The human readable function is 170 bytes, the one-liner is 104 bytes and the golfed version is 90 bytes.


Final Edit

I did some testing and here are the results:

Python 3.9.10 (tags/v3.9.10:f2f3f53, Jan 17 2022, 15:14:21) [MSC v.1929 64 bit (AMD64)]
Type 'copyright', 'credits' or 'license' for more information
IPython 7.28.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: special = {15: 'FizzBuzz', 5: 'Buzz', 3: 'Fizz'}
   ...: def fizzbuzz(number):
   ...:     for i in (15, 5, 3):
   ...:         if not number % i:
   ...:             return special[i]
   ...:     return number

In [2]: def fizz_buzz(n):
   ...:     for k, v in special.items():
   ...:         if not n % k:
   ...:             return v
   ...:     return n

In [3]: def FizzBuzz(number):
   ...:     return next((label for x, label in special.items() if not number % x), number)

In [4]: a='Fizz';b='Buzz';S={15:a+b,5:b,3:a};f=lambda n:next((l for c,l in S.items()if not n%c),n)

In [5]: fizzbuzz(255)
Out[5]: 'FizzBuzz'

In [6]: fizz_buzz(255)
Out[6]: 'FizzBuzz'

In [7]: FizzBuzz(255)
Out[7]: 'FizzBuzz'

In [8]: f(255)
Out[8]: 'FizzBuzz'
In [9]: %timeit fizzbuzz(255)
170 ns ± 0.897 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

In [10]: %timeit fizz_buzz(255)
256 ns ± 8.71 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

In [11]: %timeit FizzBuzz(255)
712 ns ± 13.1 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

In [12]: %timeit f(255)
688 ns ± 8.9 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

In [13]: import dis

In [14]: dis.dis(fizzbuzz)
  3           0 LOAD_CONST               1 ((15, 5, 3))
              2 GET_ITER
        >>    4 FOR_ITER                24 (to 30)
              6 STORE_FAST               1 (i)

  4           8 LOAD_FAST                0 (number)
             10 LOAD_FAST                1 (i)
             12 BINARY_MODULO
             14 POP_JUMP_IF_TRUE         4

  5          16 LOAD_GLOBAL              0 (special)
             18 LOAD_FAST                1 (i)
             20 BINARY_SUBSCR
             22 ROT_TWO
             24 POP_TOP
             26 RETURN_VALUE
             28 JUMP_ABSOLUTE            4

  6     >>   30 LOAD_FAST                0 (number)
             32 RETURN_VALUE

In [15]: dis.dis(fizz_buzz)
  2           0 LOAD_GLOBAL              0 (special)
              2 LOAD_METHOD              1 (items)
              4 CALL_METHOD              0
              6 GET_ITER
        >>    8 FOR_ITER                24 (to 34)
             10 UNPACK_SEQUENCE          2
             12 STORE_FAST               1 (k)
             14 STORE_FAST               2 (v)

  3          16 LOAD_FAST                0 (n)
             18 LOAD_FAST                1 (k)
             20 BINARY_MODULO
             22 POP_JUMP_IF_TRUE         8

  4          24 LOAD_FAST                2 (v)
             26 ROT_TWO
             28 POP_TOP
             30 RETURN_VALUE
             32 JUMP_ABSOLUTE            8

  5     >>   34 LOAD_FAST                0 (n)
             36 RETURN_VALUE

In [16]: dis.dis(FizzBuzz)
  2           0 LOAD_GLOBAL              0 (next)
              2 LOAD_CLOSURE             0 (number)
              4 BUILD_TUPLE              1
              6 LOAD_CONST               1 (<code object <genexpr> at 0x00000269CA7D69D0, file "<ipython-input-3-ca9b85259fb4>", line 2>)
              8 LOAD_CONST               2 ('FizzBuzz.<locals>.<genexpr>')
             10 MAKE_FUNCTION            8 (closure)
             12 LOAD_GLOBAL              1 (special)
             14 LOAD_METHOD              2 (items)
             16 CALL_METHOD              0
             18 GET_ITER
             20 CALL_FUNCTION            1
             22 LOAD_DEREF               0 (number)
             24 CALL_FUNCTION            2
             26 RETURN_VALUE

Disassembly of <code object <genexpr> at 0x00000269CA7D69D0, file "<ipython-input-3-ca9b85259fb4>", line 2>:
  2           0 LOAD_FAST                0 (.0)
        >>    2 FOR_ITER                22 (to 26)
              4 UNPACK_SEQUENCE          2
              6 STORE_FAST               1 (x)
              8 STORE_FAST               2 (label)
             10 LOAD_DEREF               0 (number)
             12 LOAD_FAST                1 (x)
             14 BINARY_MODULO
             16 POP_JUMP_IF_TRUE         2
             18 LOAD_FAST                2 (label)
             20 YIELD_VALUE
             22 POP_TOP
             24 JUMP_ABSOLUTE            2
        >>   26 LOAD_CONST               0 (None)
             28 RETURN_VALUE

In [17]: dis.dis(f)
  1           0 LOAD_GLOBAL              0 (next)
              2 LOAD_CLOSURE             0 (n)
              4 BUILD_TUPLE              1
              6 LOAD_CONST               1 (<code object <genexpr> at 0x00000269CAA370E0, file "<ipython-input-4-a067fdb714e4>", line 1>)
              8 LOAD_CONST               2 ('<lambda>.<locals>.<genexpr>')
             10 MAKE_FUNCTION            8 (closure)
             12 LOAD_GLOBAL              1 (S)
             14 LOAD_METHOD              2 (items)
             16 CALL_METHOD              0
             18 GET_ITER
             20 CALL_FUNCTION            1
             22 LOAD_DEREF               0 (n)
             24 CALL_FUNCTION            2
             26 RETURN_VALUE

Disassembly of <code object <genexpr> at 0x00000269CAA370E0, file "<ipython-input-4-a067fdb714e4>", line 1>:
  1           0 LOAD_FAST                0 (.0)
        >>    2 FOR_ITER                22 (to 26)
              4 UNPACK_SEQUENCE          2
              6 STORE_FAST               1 (c)
              8 STORE_FAST               2 (l)
             10 LOAD_DEREF               0 (n)
             12 LOAD_FAST                1 (c)
             14 BINARY_MODULO
             16 POP_JUMP_IF_TRUE         2
             18 LOAD_FAST                2 (l)
             20 YIELD_VALUE
             22 POP_TOP
             24 JUMP_ABSOLUTE            2
        >>   26 LOAD_CONST               0 (None)
             28 RETURN_VALUE

In [18]:

My original function is the fastest, it takes only about 180 nanoseconds to complete.

As a general rule of thumb, if a function generates less bytecode, it is going to cost less time, because the function would be simpler for the CPU.

Code golfing is a bad thing, you use less characters but the logic is poorly thought because you can only pack so many ideas inside a few characters, so the compiler generates more bytecode to compensate.

And I don't use generator expressions, as you can see, generator expressions generate more code-objects which means you have more bytecode which in turn means performance degradation (very broadly speaking), because function calls are expensive.

\$\endgroup\$
5
  • \$\begingroup\$ Note that I wrote all of this on an Android phone and I don't have physical access to computers in nights (it was nighttime at the time of writing in my locale), I won't give you more details, the point is I wrote all of this without running or testing any of the code, but I know exactly what output it will have and how it will be executed. \$\endgroup\$ Apr 23, 2022 at 15:16
  • 1
    \$\begingroup\$ Would be better to iterate over the constant (which should be named as a constant as well) rather than hardcoding (15, 5, 3). Optionally -- and this is no better or worse, but it does fit within the OP's theme of evaluating tradeoffs between code readability and code compactness or even "coolness" -- it can also be expressed as return next((label for x, label in SPECIAL.items() if not number % x), number). \$\endgroup\$
    – FMc
    Apr 23, 2022 at 15:22
  • 1
    \$\begingroup\$ @FMc I know I can write like that, but I am posting an answer to a question asked by a beginner and I am trying to keep the answer as human readable as possible... \$\endgroup\$ Apr 23, 2022 at 15:25
  • \$\begingroup\$ @ΞένηΓήινος thanks for the answer, that makes a lot of sense. I'm not quite sure if you were being sarcastic with all the very obvious stuff like what return is and what a dictionary is. But regardless, I appreciate the constructive criticism. The solution I went with was sort of just planned poorly on the spot. Using the basis I started on, I couldn't really remove the redundant iterations without reworking the solution. I did actually make something very similar to this about 10 mins after posting. Thankyou! \$\endgroup\$
    – Dat Boi
    Apr 23, 2022 at 16:20
  • \$\begingroup\$ Your image of text isn't very helpful. It can't be read aloud or copied into an editor, and it doesn't index very well. Please edit your post to incorporate the relevant text directly (preferably using copy+paste to avoid transcription errors). \$\endgroup\$ Apr 24, 2022 at 9:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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