patternjavaMajor
First solution to FizzBuzz
Viewed 0 times
solutionfirstfizzbuzz
Problem
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 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 {
Solution
System.out.printf("%4.8s ", ...);is actually not necessary here: this code means that you will format yourStringso 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 theStringwith no change. Therefore, it is simpler to haveSystem.out.print(... + " ");.
- 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.
- 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();
}
}Code Snippets
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();
}
}Context
StackExchange Code Review Q#121387, answer score: 27
Revisions (0)
No revisions yet.