First solution to FizzBuzz

Question

I'm a first year college student, Computer Science major. This is my crack at the FizzBuzz interview question in Java. What kind of improvements could I make?

//Prints numbers 1 - 100, 25 values per line.
//Numbers that are multiples of 3 are replaced with Fizz
//Numbers that are mulitples of 5 are replaced with Buzz
//Numbers that are multiples of both 3 and 5 are replaced with FizzBuzz

public class FizzBuzz {

    public static void main(String[] args) {

        String fizz = "Fizz";
        String buzz = "Buzz";
        String fizzBuzz = "FizzBuzz";

        //Loop from 1 through 100
        for (int i = 1; i <= 100; i++) {

            if (i % 3 == 0 && i % 5 == 0) {          //Checks for numbers that are both multiples of 5 and 3
                if (i % 25 != 0) {                   //Control flow allows 25 values printer per line
                    System.out.printf("%4.8s ", fizzBuzz);                  
                }
                else {
                    System.out.printf("%4.8s\n ", fizzBuzz); 
                    System.out.println();
                }

            }

            else if (i % 3 == 0) {                   //Checks for numbers that are multiples of 3
                if (i % 25 != 0) {                   //Control flow allows 25 values printer per line
                    System.out.printf("%4.8s ", fizz);
                }
                else {
                    System.out.printf("%4.8s\n ", fizz);
                    System.out.println();
                }

            }

            else if (i % 5 == 0) {                   //Checks for numbers that are multiples of 5
                if (i % 25 != 0) {                   //Control flow allows 25 values printer per line
                    System.out.printf("%4.8s ", buzz);
                }
                else {
                    System.out.printf("%4.8s\n ", buzz);
                    System.out.println();
                }

            }

            else {                                   //Prints numbers that are not multiples of 3 or 5
                if (i % 25 != 0) {                   //Control flow allows 25 values printer per line
                    System.out.print(i + " ");
                }
                else {
                    System.out.println(i + " ");
                    System.out.println();
                }
            }
        }
    }
}

Answer 1

1. System.out.printf("%4.8s ", ...); is actually not necessary here: this code means that you will format your String so that it has at minimum 4 characters, at most 8. Since "Fizz", "Buzz" and "FizzBuzz" have a length between 4 and 8, this will result in just outputing the String with no change. Therefore, it is simpler to have System.out.print(... + " ");.
2. You have one part of your code that is repeated 4 times. Code duplication is something you want to avoid at all costs. It is better to refactor this duplicated logic into a small reusable method that you can then call elsewhere.
3. In the same way, you can store inside two booleans the result of whether the number of divisible by 3 and 5.

In this case, you are repeating the check to print only 25 values per line, when you could do this only a single time after all the if/else logic.

You can then write your code a lot more simply:

for (int i = 1; i <= 100; i++) {
    boolean shouldFizz = i % 3 == 0;
    boolean shouldBuzz = i % 5 == 0;
    if (shouldFizz && shouldBuzz) { // Checks for numbers that are both multiples of 5 and 3
        System.out.print(fizzBuzz);
    } else if (shouldFizz) { // Checks for numbers that are multiples of 3
        System.out.print(fizz);
    } else if (shouldBuzz) { // Checks for numbers that are multiples of 5
        System.out.print(buzz);
    } else { // Prints numbers that are not multiples of 3 or 5
        System.out.print(i);
    }
    System.out.print(" ");
    if (i % 25 == 0) { // Control flow allows 25 values printer per line
        System.out.println();
        System.out.println();
    }
}

Answer 2

Two of the key sins in software engineering are scope creep and gold plating. Scope creep is a sin committed by clients and managers. Gold plating is a sin committed by programmers. This is a perfect example of the latter. The requirements for fizz-buzz are very simple. There is nothing in those requirements regarding printing 25 values per line. An implementation of fizz-buzz that prints one value per line is just fine.

While Java is well known for being a long-winded language, even in Java, an implementation of fizz-buzz that is 63 lines long (counting blank lines and comments) is far too long. As an interviewer, I would congratulate you on getting the answer right. However, in the post-interview meeting, I would most likely recommend that you not be hired.

Even in Java, the fizz-buzz problem should be very small (allowing a couple of violations of very standard coding practices):

// Solve the fizz-buzz problem: See
// http://imranontech.com/2007/01/24/using-fizzbuzz-to-find-developers-who-grok-coding/
public class FizzBuzz {
    public static void main(String[] args)
    {
        for (int i = 1; i <= 100; i++)
            if (i % 15 == 0) System.out.println("FizzBuzz");
            else if (i % 3 == 0) System.out.println("Fizz");
            else if (i % 5 == 0) System.out.println("Buzz");
            else System.out.println(i);
    }
}

Even after incorporating those very standard coding practices (the statements that follow for, if, else, etc. should be on separate lines and should be enclosed by curly braces), fizz-buzz should be small, even in Java.

Answer 3

_{Disclaimer: Generalizing FizzBuzz might not be for the faint of heart, and as such David Hammen has valid points related to scope creep and gold plating. Other answers focus on possible simplifications and other aspects, but in this answer I would like to go down the path of generalizing the FizzBuzz algorithm allowing for other numbers and additional words.}

There are two major improvement I would make to your code:

• When recognizing that your print statements are very equal, extract these to a single print at end of method so that you don't repeat your self
• Similar, when recognizing that 15 is the multiple of 3 and 5, consider changing to only calculate the modulo's a minimum number of times. That is change your code to allow for memorising for the case of "FizzBuzz"

Generic FizzBuzzRule class

To help us achieve this goal, here is some code for keeping lists of FizzBuzz rules:

class FizzBuzzRule {
    public int value;
    public String string;

    public FizzBuzzRule(int value, String string) {
        this.value = value;
        this.string = string;
    }

    // Define default set
    static public final ArrayList<FizzBuzzRule> BASIC_RULES = new ArrayList<>();
    static {
        BASIC_RULES.add(new FizzBuzzRule(3, "Fizz"));
        BASIC_RULES.add(new FizzBuzzRule(5, "Buzz"));  
    }

A fizzyfy method

This code also defines the basic set as a static so that we can access the basic set of rules using FizzBuzzRule.basicRules.

Further to simplify the generation of the list we'll implement a dedicated method to fizzyfy a number with a give rule set, which can look like this:

public static String fizzyfy(int i, ArrayList<FizzBuzzRule> rules) {

    StringBuilder fizzy = new StringBuilder();

    for (FizzBuzzRule rule : rules) {
        if (i % rule.value == 0) {
            fizzy.append(rule.string);
        }
    }

    if (fizzy.length() > 0) {
        return fizzy.toString();
    } else {
        return Integer.toString(i);
    }
} 

Notice how we now use a StringBuilder to concatenate the different part of the fizzyfied number. This make it a lot more generic, and in addition it only calculates any modulo operation just once for a given set. Instead of doing it for 3, 5 and 15, it only does it for 3 and 5 in the basic version. This makes even more sense if you add more rules like printing "Baz" if dividable by 7, and "Shoe" if divisable by 11... This would in your code make a lot of if statements.

In this basic extension I use the modulo operation, i.e. i % rule.value to calculate the remainder of the division i / rule.value. Due to use of a class this allows for the extension to add different rules for other numbers and/or words. An alternate ruleset (inspired by Wikipedia could be defined as:

ArrayList<FizzBuzzRule> extendedRuleset = new ArrayList<>();
extendedRuleset.append(new FizzBuzzRule(3, "Fizz");
extendedRuleset.append(new FizzBuzzRule(5, "Buzz");
extendedRuleset.append(new FizzBuzzRule(7, "Pop");
extendedRuleset.append(new FizzBuzzRule(11, "Whack");

Another extension, inspired by Twenty ways to FizzBuzz (in Javascript), could be to take it to the extreme and introduce predicates (as in #12) in to the generic rule class. However I'm going to leave that for another post sometime in the future.

Rebuild the main loop

I see the argument for using System.out.printf() as you can limit the output and get nice columns. However here also you should try to avoid repeating your self, and you should use a format which makes more sense related to the present output.

To avoid repeating yourself I would make the end of line a separate statement, and when combined with the other elements we can end with something like:

public static void main(String[] args) {

    for (int i = 1; i <= 21; i++) {

        System.out.printf("%-8.8s ", fizzyfy(i, FizzBuzzRule.BASIC_RULES));

        if (i % 4 == 0) {
            System.out.println();
        }
    }
}

Notice how this code limits the string to be of exactly 8 characters long, and it adds an extra space as column separator. I opted for only 4 numbers in each column, but change that at will.

Another advantage of this approach is that you know have separated the business logic and presentation logic. The business logic is split into the rules definitions of FizzBuzzRule and "fizzyfication" through fizzyfy(), and finally the presentation logic is neatly grouped within the main() method.

If you want to, you could of course opt for having the fizzyfy() within the FizzBuzzRule class, and choose other names here and there. But still the general principle of not repeating yourself, and still allowing for modifications and extending the ruleset or presentation is maintained.

To use the alternate ruleset, extendedRuleset, replace the second argument of fizzyfy() from FizzBuzzRule.BASIC_RULES to extendedRuleset (and add the creation of the ruleset in front of the for loop).

Answer 4

public class FizzBuzz {
    public static void main(String[] args) {
        for (int i = 1; i <= 100; i++) {
            if (i % 15 == 0) {
                System.out.print("FizzBuzz");
            } else if (i % 3 == 0) {
                System.out.print("Fizz");
            } else if (i % 5 == 0) {
                System.out.print("Buzz");
            } else {
                System.out.print(i);
            }


            if ( i % 25 == 0 ) { 
                System.out.println(); 
            } else {
                // Separate our results.
                System.out.print(" ");
            }
        }
    }
}

This is a very simple white board question as such, the examiners asking this will most likely not be looking for modularity, so there is no point in storing local variables that are being initialized to a predetermine value, unless we were going to be modifying them, based off of the original question and post we aren't actually modifying them.

So I removed the stored string variables. Most results will in the end do a check for i % 5 && I % 3 to check if a resulting answer should be FizzBuzz you can simplify this with getting the lowest possible number they can both modulate into... simply put that is 3 * 5.

Another possible solution is doing 3 if checks without any elses they are

if(i % 3 == 0) System.out.print("Fizz");
if(i % 5 == 0) System.out.print("Buzz");
if(i % 3 != 0 && i % 5 != 0) System.out.print(i);

The issue with this solution is every single iteration we are doing 3 modulations, and 3 possible checks. That are not required. using the if and else if sequence in this results with best case scenario of only doing 1 modulation to determine the answer, worst case 3. This saves us some time.

Below I am doing the if i % 25 == 0 check to determine if we need a new line if we dont I am adding a space to the print out so we can actually see each individual results.

I would say the original post isn't a bad solution for a first attempt. just something to keep in mind if you have something that you are doing multiple times that is more then 3 or 4 lines of code there should be a way to encapsulate that, I say that with regards to the:

if (i % 25 != 0) {                   //Control flow allows 25 value printer per line
    System.out.printf("%4.8s ", buzz);
}
else {
    System.out.printf("%4.8s\n ", buzz);
    System.out.println();
}

Answer 5

You basically repeat the same 7 lines 4 times. And what's worse is that, as written, half the time the full block isn't possible.

if (i % 25 != 0) {
    System.out.printf("%4.8s ", fizzBuzz);                  
}
else {
     System.out.printf("%4.8s\n ", fizzBuzz); 
     System.out.println();
 }

This is totally unnecessary on the %3 and number branch as those can never be executed when %25. It is also unnecessarily long.

System.out.printf("%4.8s ", fizzBuzz);   
if (i % 25 == 0) {
     System.out.println();
 }

Now the 7 lines are 4 lines, and it should also now be obvious that a further reduction can be made -- just do the final 3 lines once, outside of the conditions that determine what to print. Whether you print a newline or not, is unconnected to what you print, there's no reason to combine them.

Fixing the above moves it out of the "works but horrible implementation" category.

But as a general rule, I dislike mixing output and logic, and to a lesser extent loops and logic.

If you extract the logic in the loop out into it's own function, that returns a string, you make it easier to read, maintain and test.

Answer 6

Most of these answers, as well as the original proposed in the question, seem overly complex. A program producing the correct output should be about ten lines:

    public class fizzbuzz {

        public static void main(String[] args) {

            for (int num = 1; num < 101; num++) {

                if ( num % 3 == 0 ) { System.out.printf("Fizz"); }
                if ( num % 5 == 0 ) { System.out.printf("Buzz"); }
                if ( num % 3 != 0 && num % 5 != 0 ) { System.out.printf("%d ", num); }
                if ( num % 3 == 0 || num % 5 == 0 ) { System.out.printf(" "); }
                if ( num % 25 == 0 ) { System.out.println(); }

            }
        }
    }

The code presented simply loops through the range of 1-100. I use '< 101' rather than '<= 100' to simplify the loop comparison. The five IF statements simply check the current number, and print whatever is appropriate. Other solutions may use more complex nested IF statements, but my experience, and that of other programmers is that nested loops are often harder to follow and lead to more logic errors. If they can be avoided, they generally should. Other than that the program should be fairly self explanatory.

Answer 7

You could do two (2) small loops to do this; first one will fill out an array and the second one will print 25 array cells per line.

public static void main(String args[]) {

    String[] storage = new String[101];

    // Store data
    for(int i = 1; i <= 100; i++) {

        if (i % 3 == 0 && i % 5 == 0) {
            storage[i] = "FizzBuzz";
        }
        else if (i % 3 == 0) {
            storage[i] = "Fizz";
        }
        else if (i % 5 == 0) {
            storage[i] = "Buzz"; 
        }
        else {
            storage[i] = Integer.toString(i);
        }

    }

    // Print data
    for(int i = 1; i < storage.length; i++) {
        System.out.print(storage[i] + " ");
        if(i % 25 == 0) {
            System.out.print("\n");
        }
    }
}

This is a little bit more code than required for the simplest and most straight forward solution, but with an array, the data would be reusable. Maybe you want to print it backwards? It's silly to do the same processing over and over again.

But in-case you absolutely know you will not use the data again, you can just print it out with maximum of 5 comparison operations per iteration:

public static void main(String args[]) {
    for(int i = 1; i <= 100; i++) {
        if (i % 3 == 0 && i % 5 == 0) {
            System.out.print("FizzBuzz ");
        }
        else if (i % 3 == 0) {
            System.out.print("Fizz ");
        }
        else if (i % 5 == 0) {
            System.out.print("Buzz "); 
        }
        else {
            System.out.print(i + " ");
        }
        if(i % 25 == 0) {
            System.out.print("\n");
        }
    }
}

Answer 8

public class FizzBuzz{ 
  public final int FIZZ = 3;
  public final int BUZZ = 5;

  private String getFizzyBuzzyString(int num) {
    String fizzybuzzyString = "";
    fizzyBuzzyString += (num % FIZZ == 0)? "Fizz" : "";
    fizzyBuzzyString += (num % BUZZ == 0)? "Buzz" : "";

    return StringUtils.hasLength(fizzyBuzzyString)? fizzyBuzzyString : String.valueOf(num);
  }

   public static void main(String [] args){
     for (int i = 1; i < 101; i++){
       System.print(getFizzyBuzzyString(i) + " ");
       if (i % 25 == 0) {
         System.print("\n"); 
       }
    }
 }

Improvement

This moves the fizzbuzz logic away from the main method; a good thing to do for code interviews because it shows that you understand Separation of Concerns Your fizzbuzz method now only tells what the integer should be printed as), and the printing itself is handled by the main method.

Sidenote (irrelevant for most code interviews)

There is a con to this approach. This will be very slightly slower than simply having 4 if-clauses; the former checks divisibility by 3 first and then by 5 while the latter approach simply checks for 15 (see shortcuting in if statements). However, due to the fact that most numbers are not divisible by 15, the speed difference is miniscule in terms of complexity.