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;
}

Answer 1

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);
    }
}

Answer 2

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");
    }
}

Answer 3

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);

Answer 4

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.

Answer 5

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.