Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This questions is originally from http://contestcoding.wordpress.com/2013/06/28/fizz-buzz-bizz-fuzz/.

  • Print the integers from 1 to 100,
  • but for the multiples of 3, print "Fizz" instead and
  • for multiples of 5, print "Buzz".
  • If the number contains a 3 (for example 23), print "Bizz" and
  • if the number contains a 5, print “Fuzz”
  • (if it contains multiple 3s or 5s, just print one "Bizz" or "Fuzz").
  • If the number contains more than one of these attributes, print every word (for example 33 prints "FizzBizz", as 33 is both a multiple of 3 and contains the digit 3).

My Java code for the same is:

public class FizzBuzz {
    public static void main(String[] args) {
        for (int i = 1; i < 101; i++) {
            // Set this to true when one of the special conditions is met.
            boolean printed = false; 

            if (i % 3 == 0) {
                // When i is divisible by 3, then print "Fizz"
                printed = true;
                System.out.print("Fizz");
            } else if (i % 5 == 0) {
                // When i is not divisible by 3 but is divisible by 5, then print "Buzz"
                printed = true;
                System.out.print("Buzz");
            }

            if (Integer.valueOf(i).toString().indexOf("3") != -1) {
                // When i has the digit 3 in it, then print "Bizz"
                printed = true;
                System.out.print("Bizz");
            } else if (Integer.valueOf(i).toString().indexOf("5") != -1) {
                // When i has the digit 5 in it, then print "Fuzz"
                printed = true;
                System.out.print("Fuzz");
            }

            if (printed == false) {
                // The number does not satisfy any of the special conditions above.
                System.out.print(i);
            }
            System.out.println();
        }
    }
}

Please provide code review comments for the same.

share|improve this question
    
Wouldn't it simply make the most sense to have an acceptance test in place before just jumping right to code? As with the original fizz buzz, you should be able to know exactly what all of the lines of the output should be in advance, and then craft your program to match that test. –  Kzqai Dec 27 '13 at 18:45

2 Answers 2

up vote 7 down vote accepted

Overall, this is a pretty straightforward program. See the bottom of my answer for an Extreme Makeover: Code Edition of the program.


You have a hard-coded "magic number" in your for loop. It would be better to use a variable.

for (int i = 1; i < 101; i++) // not the best

int num = 101;
for (int i = 1; i < num; i++); // better

Your if test conditions can be shortened a bit.

if (Integer.toString(i).indexOf("3") != -1)

Your logic is a bit off. You should actually have fewer else if conditions (this is rarely the case, but here it is applicable). For example, when i reaches "15", is should print "FizzBuzzFuzz", but your program only prints "FizzFuzz".


Final code:

public class Test
{
    public static void main(String... args)
    {
        int num = 101;
        for (int i = 1; i < num; i++)
        {
            boolean printed = false;

            if (i % 3 == 0)
            {
                printed = true;
                System.out.print("Fizz");
            }
            if (i % 5 == 0)
            {
                printed = true;
                System.out.print("Buzz");
            }

            if (Integer.toString(i).indexOf("3") != -1)
            {
                printed = true;
                System.out.print("Bizz");
            }
            if (Integer.toString(i).indexOf("5") != -1)
            {
                printed = true;
                System.out.print("Fuzz");
            }

            if (printed == false) System.out.print(i);
            System.out.println();
        }
    }
}

Extreme Makeover: Code Edition

Let's use a StringBuilder and some ternary operators. And let's get rid of that boolean.

public class Test
{
    public static void main(String... args)
    {
        int num = 101;
        for (int i = 1; i < num; i++)
        {
            StringBuilder sb = new StringBuilder();
            if(i % 3 == 0) sb.append("Fizz");
            if(i % 5 == 0) sb.append("Buzz");

            if(Integer.toString(i).indexOf("3") != -1) sb.append("Bizz");
            if(Integer.toString(i).indexOf("5") != -1) sb.append("Fuzz");

            if (sb.length() == 0) System.out.print(i);
            else System.out.print(sb);
            System.out.println();
        }
    }
}
share|improve this answer
    
if you read the instructions OP gave again, the answer should be FizzFuzz –  Malachi Dec 27 '13 at 17:44
    
@Malachi For which one? –  syb0rg Dec 27 '13 at 17:44
    
@syb0rg, I think that I may have read it incorrectly. –  Malachi Dec 27 '13 at 17:47
    
@Malachi I did the same at first. I had to go back and revise the logic in the code so that it would fit all of the rules. –  syb0rg Dec 27 '13 at 17:48
    
Why use a ternary operator instead of just a single-line if statement: if (Integer.toString(i).IndexOf("3") != -1) sb.Append("Bizz"); Unless Java's StringBuilder works different from what I'd expect, the return value is just for fluent chaining, but you don't have to assign it. –  Anthony Dec 27 '13 at 19:50
  1. Your implementation is broken, for example 15 should print FizzBuzzFuzz but yours will print FizzFuzz. 35 should print BuzzBizzFuzz but yours will print BuzzBizz.

  2. Turning an integer into a string can be done with Integer.toString(i).

  3. For reusability your code should be encapsulated in a class which you can instantiate and play as many games as you like.

  4. Once you have refactored the code into a class consider decoupling logic from output. For example you can pass in an Appendable to which you can append your output without having to care where it ends up.

share|improve this answer
    
Hi ChrisWue, Based on my understanding of the question: The first two conditions (divisibility rules) are mutually exclusive of each other. Similarly, the next two conditions (presence of 3 and 5 in the number) are mutually exclusive. I can see that the question can be read so too. However, when I contacted the original question owner, he felt that my answer was correct. –  Karthick S Dec 27 '13 at 17:36
    
In addition, thanks a lot for your other comments. –  Karthick S Dec 27 '13 at 17:37
    
@KarthickS: Well the requirements state If the number contains more than one of these attributes, print every word so the spec is ambiguous at least. –  ChrisWue Dec 27 '13 at 18:48
    
Hi Chris, the original blog post owner has confirmed what you mentioned here. It is indeed a mistake. Thanks a lot for pointing it out. –  Karthick S Jan 2 at 10:04

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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