patternjavaModerate
Efficient FizzBuzz
Viewed 0 times
efficientfizzbuzzstackoverflow
Problem
I wrote the following Fizz Buzz program. How can I improve it or make it more efficient?
public class FizzBuzz
{
public static void main(String[] args)
{
System.out.println("--------Fizz Buzz program-------------");
for(int num=1;num<=100;num++)
{
if(num%3==0)
{
if(num%5==0)
System.out.println("FizzBuzz");
else
System.out.println("Fizz");
}
else if(num%5==0)
{
if(num%3==0)
System.out.println("FizzBuzz");
else
System.out.println("Buzz");
}
else
System.out.println(num);
}
}
}Solution
The issues I see with your code are:
-
Brace style: Leaving out the optional braces can lead to bugs when code is modified later. If you want to omit the braces, then also put the body on the same line to avoid any misconceptions.
Personally, I would advise that you stick with the standard Java conventions to make everyone's life easier. It also relieves the temptation to skimp on braces in the first place.
By the way, opening braces on a separate line can be brutal when it comes to try-catch-finally blocks.
-
Repetition of
- Indentation: Your indentation is inconsistent. One space per level is insufficient for readability.
-
Brace style: Leaving out the optional braces can lead to bugs when code is modified later. If you want to omit the braces, then also put the body on the same line to avoid any misconceptions.
if (num % 3 == 0)
{
if (num % 5 == 0) System.out.println("FizzBuzz");
else System.out.println("Fizz");
}Personally, I would advise that you stick with the standard Java conventions to make everyone's life easier. It also relieves the temptation to skimp on braces in the first place.
if (num % 3 == 0) {
if (num % 5 == 0) {
System.out.println("FizzBuzz");
} else {
System.out.println("Fizz");
}
}By the way, opening braces on a separate line can be brutal when it comes to try-catch-finally blocks.
try
{
…
}
catch (Exception e)
{
…
}
finally
{
…
}- Redundancy: The multiple-of-15 case is handled twice; it should need to be handled once.
-
Repetition of
System.out.println(): I suggest using a ternary conditional expression instead.for (int num = 1; num <= 100; num++) {
System.out.println((num % 15 == 0) ? "FizzBuzz" :
(num % 3 == 0) ? "Fizz" :
(num % 5 == 0) ? "Buzz" :
num);
}Code Snippets
if (num % 3 == 0)
{
if (num % 5 == 0) System.out.println("FizzBuzz");
else System.out.println("Fizz");
}if (num % 3 == 0) {
if (num % 5 == 0) {
System.out.println("FizzBuzz");
} else {
System.out.println("Fizz");
}
}try
{
…
}
catch (Exception e)
{
…
}
finally
{
…
}for (int num = 1; num <= 100; num++) {
System.out.println((num % 15 == 0) ? "FizzBuzz" :
(num % 3 == 0) ? "Fizz" :
(num % 5 == 0) ? "Buzz" :
num);
}Context
StackExchange Code Review Q#36708, answer score: 15
Revisions (0)
No revisions yet.