Modifying a function without losing sense

Question

Edit: I think I need to be more precise, the code itself is good but I need to modify it not to have a block with more than 3 branches. For instance, one could separate it by doing multiple functions

I am trying to do the Fizzbuzz, my code is right but the problem is that I cannot have more than three branches in a conditional block, here is what I've done so far:

#include <stdio.h>
#include <unistd.h>

void fizzbuzz(int number_one, int number_two)
{
    for (int nbr = number_one; nbr <= number_two; nbr++) {
        if (nbr % 15 == 0)
            printf("Fizzbuzz\n");
        else if (nbr % 5 == 0)
            printf("Buzz\n");
        else if (nbr % 3 == 0)
            printf("Fizz\n");
        else
            printf("%i\n", nbr);
    }
}

int main(int ac, char **av)
{
    if (ac == 1)
        return 84;
     if (ac == 3) {
        int number_one = atoi(av[1]);
        int number_two = atoi(av[2]);
        if (number_one > number_two) {
            printf("Error: the second parameter must");
            printf("be greater than the first one.\n");
            return 84;
        } else
            fizzbuzz(number_one, number_two);
    }
    return 0;
}

If you add `continue;` after each of the conditional `printf` calls you don't need an else, it can just be unconditional. — Retired Ninja, Jan 18 '23 at 12:29
Do you mean by *"three branches in a conditional block"* `if (c1) {} else if (c2) {} else {}`? — Zakk, Jan 18 '23 at 12:53
To have more than one statement in a case you need to enclose them in curly braces to create a block of code this way — Will Ness, Jan 18 '23 at 12:55
Branches in the disassembly (sensible) or branches in the C code (less sensible)? — Lundin, Jan 18 '23 at 13:54

score 2 · Answer 1 · answered Jan 18 '23 at 14:59

I'm posting this evil-looking code just to illustrate the problem with artificial requirements and "pre-mature" optimization. Disclaimer: DO NOT WRITE CODE LIKE THIS.

#include <stdbool.h>
#include <stdint.h>

static void fizzbuzz (uint_fast16_t start, uint_fast16_t end)
{
  for(uint_fast16_t i=start; i<=end; i++)
  {             //012345678  
    char str[] = "FizzBuzz\0";
    char* strp = str;
    bool div3 = (i % 3)==0;
    bool div5 = (i % 5)==0;

    strp[(9 - 5*div3)] = '\0';
    strp[(9 - 5*(div3 & div5))] = 'b';
    strp += 4 * div5 * !div3;
    str[0] *= div3 | div5;

    char fmt[] = "%d";
    fmt[0] -= div3 | div5;
    fmt[0] *= !div3 * !div5;
    printf(fmt, i);
    puts(strp);
  }
}

Apart from the loop condition, it contains zero branches in the C code. But still quite a few in the generated machine code. Is it more efficient than what you already have? I kind of doubt it. At a glance, the machine code might be ever so slightly more efficient. At the cost of throwing readability out the window.

Notably the massive bottlenecks in this code isn't the visible or generated branches, but the printf calls. So it's pretty senseless to do pre-mature optimization like this while keeping user output and the algorithm mixed. And depending on target, branches may or may not be a problem at all. Low- to mid-range CPUs don't have branch prediction or instruction cache.

Some lessons learnt:

Always question if the specification makes sense. The above code sates your specification "no more than 3 branches" perfectly, yet it's horrible to read and maintain. And not necessarily much faster either.
Focus on writing code as readable as possible.
Only manually optimize with a specific system in mind and only when you have a known bottleneck somewhere in your code.
Don't mix UI and algorithms, keep them separated.

This: https://godbolt.org/z/PreoszTKv might be more dangerous, beeing slightly more readable. — Bob__, Jan 18 '23 at 15:19

Vlad from Moscow · Answer 2 · 2023-01-19T17:57:26.160

The function can be defined the following way

void fizzbuzz(int number_one, int number_two)
{
    if ( !( number_two < number_one ) )
    {    
        do
        {
            if ( !( ( number_one % 3 == 0 ) || ( number_one % 5 == 0 ) ) )
            {
                printf( "%d", number_one );
            }     

            if ( number_one % 3 == 0 )
            {
                printf( "Fizz");
            }

            if ( number_one % 5 == 0 )
            {   
                printf("Buzz");
            }

            putchar ( '\n' );
        } while ( number_one++ != number_two );
    }
}

Pay attention to that you should not use the for loop. For example if number_two is equal to INT_MAX you will have an infinite loop or undefined behavior.

Also it would be better to use named constants instead of the magic number 3 and 5. In this case to change the divisors in the function it will be enough to change only one declaration. For example

void fizzbuzz(int number_one, int number_two)
{
    const int first_divisor = 3, second_divisor = 5;

    if ( !( number_two < number_one ) )
    { 
        do
        {
            if ( !( ( number_one % first_divisor == 0) || ( number_one % second_divisor == 0 ) ) )
            {
                printf( "%d", number_one );
            }     

            if ( number_one % first_divisor == 0 )
            {
                printf( "Fizz");
            }

            if ( number_one % second_divisor == 0 )
            {   
                printf("Buzz");
            }

            putchar ( '\n' );
        } while ( number_one++ != number_two );
    }
}

Here is a demonstration program.

#include <stdio.h>
#include <limits.h>

void fizzbuzz( int number_one, int number_two )
{
    const int first_divisor = 3, second_divisor = 5;

    if (!( number_two < number_one ))
    {
        do
        {
            if (!( ( number_one % first_divisor == 0 ) || ( number_one % second_divisor == 0 ) ))
            {
                printf( "%d", number_one );
            }

            if (number_one % first_divisor == 0 )
            {
                printf( "Fizz" );
            }

            if (number_one % second_divisor == 0 )
            {
                printf( "Buzz" );
            }

            putchar( '\n' );
        } while (number_one++ != number_two);
    }
}

int main( void )
{
    fizzbuzz( INT_MAX - 20, INT_MAX );
}

The program output is

2147483627
Fizz
2147483629
Buzz
Fizz
2147483632
2147483633
Fizz
Buzz
2147483636
Fizz
2147483638
2147483639
FizzBuzz
2147483641
2147483642
Fizz
2147483644
Buzz
Fizz
2147483647

Try this call

fizzbuzz( INT_MAX - 20, INT_MAX );

with other presented programs in answers where there is used the for loop.:)

If you want to output "Fizzbuzz" instead of "FizzBuzz" then the function can look for example the following way

void fizzbuzz( int number_one, int number_two )
{
    const int first_divisor = 3, second_divisor = 5;

    if (!( number_two < number_one ))
    {
        do
        {
            if (!( ( number_one % first_divisor == 0 ) || ( number_one % second_divisor == 0 ) ))
            {
                printf( "%d", number_one );
            }

            if (number_one % first_divisor == 0 )
            {
                printf( "Fizz" );
            }

            if (number_one % second_divisor == 0 )
            {
                printf( number_one % first_divisor == 0 ? "buzz" : "Buzz" );
            }

            putchar( '\n' );
        } while (number_one++ != number_two);
    }
}

`FizzBuzz` is not `Fizzbuzz` though. The former is a much easier problem to solve. — Lundin, Jan 18 '23 at 14:14
When i%3==0 and i%5==0 you are supposed to write `Fizzbuzz` with the b in lower case. — Lundin, Jan 18 '23 at 14:23
@Lundin In the provided link in the question there is outputted FizzBuzz instead of Fizzbuzz.:) — Vlad from Moscow, Jan 18 '23 at 14:28

amirhm · Answer 3 · 2023-01-18T13:34:41.827

something like this :

void fizzbuzz(int number_one, int number_two)
{
    for (int nbr = number_one; nbr <= number_two; nbr++) {
        int divisible = 0;
        if (nbr % 3 == 0){
             printf("Fizz"); divisible = 1;
        }
        if (nbr % 5 == 0) {   
            printf("Buzz");divisible = 1;
        }
        if (divisible == 0)
            printf("%i", nbr);
        printf("\n");
    }
}

score 1 · Answer 4 · answered Jan 18 '23 at 12:48

1

Perhaps:

if (num % 3 == 0) {
    printf("fizz");
    if (num % 5 == 0) {
       printf("buzz");
    }
} else if (num % 5 == 0) {
    printf("buzz")
} else {
   printf("%d", num);
}

printf("\n);

answered Jan 18 '23 at 12:48

Harith

4,663
1
5
20

This I can understand. – Will Ness Jan 18 '23 at 12:57
@WillNess In fact there are more than tree branches.:) – Vlad from Moscow Jan 18 '23 at 13:02

Costantino Grana · Answer 5 · 2023-01-21T20:34:57.140

0

Everybody loves Fizzbuzz. But I think this is one of the most notable solutions (which correctly prints the "Fizzbuzz" instead of "FizzBuzz"):

#include <stdio.h>

void fizzbuzz(int number_one, int number_two)
{
    char *fmt[] = { [0] = "Fizzbuzz\n", [1] = "%d\n", [6] = "Fizz\n", [10] = "Buzz\n" };
    for (int i = number_one; i <= number_two; ++i) {
        int k = i % 15;
        printf(fmt[k * k * k * k % 15], i);
    }
}

Discrete mathematics is a really tough topic, I'm still too ignorant about.

edited Jan 21 '23 at 20:34

answered Jan 19 '23 at 12:32

Costantino Grana

3,132
1
15
35

Yeah, I wonder how many kids could play with [this](https://math.stackexchange.com/questions/14084/proof-of-the-euler-generalisation-of-fermats-little-theorem-using-modular-arith)... Not [many](https://godbolt.org/z/9bcfMYK4a), I guess ;). – Bob__ Jan 21 '23 at 14:19
@Bob__ Argh... I forgot about overflow... But modular arithmetic [comes to the rescue again](https://godbolt.org/z/GhGvnh3PM)! – Costantino Grana Jan 21 '23 at 20:37

Modifying a function without losing sense

5 Answers5