FizzBuzz Solution Rags to Different Rags [closed]

Question

I took some code and reviewed it, but it got me thinking, so I re wrote the code a little bit and would like to ask what you think of it and why it's bad to do this, even though it works.

public class FizzBuzz{
    public static void main(String[] args){
        for(int i = 1 ; i < 101; i++)
            if(i % (5*3) == 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);
    }
}

I removed braces because these are all essentially one-line statements and don't need them, so what does this say about my Java coding style?

Answer 1

Taking your question at face value:

I removed braces because these are all essentially one-line statements and don't need them, so what does this say about my Java coding style?

They are not essentially 1-line statements. Your assumptions are incorrect.

An if is a statement, and a System.out.println is another statement. Thus, you have two statements there... now, the code segment:

        if(i % (5*3) == 0)
            System.out.println("FizzBuzz");

represents two statements on two lines... but, the statement:

       else if(i % 3 == 0)

technically merges one if statement (with an else clause) in to a second if statement. You thus have two statements on one line.

What conclusion can you draw from there? That code style does not depend on the number of statements on one line, because it is normal, and acceptable to have an else and a new if on a single line.

Thus, the fact that it is a 1-liner or not is not the issue. The issue relates to readability, and maintainability.

Your code is readable, sure, but is it maintainable? No.

Maintenance requires making changes. Now, the base reason I have against the un-braced 1-liners, is that maintenance often requires breaking a single statement in to multiple.

Thus, a line like:

        if(i % (5*3) == 0)
            System.out.println("FizzBuzz");
        else if (....)

may need to become:

        if(i % (5*3) == 0) {
            System.out.println("FizzBuzz");
            dualmatch++;
        } else if (....)

Note, now, that to make a single change to count the number of times that the double-modulo has happened (FizzBuzz), we have added a single line of code, but also modified 2 other lines.

Thus, we have had to modify two lines, and add a third, just to add one line.

Now, what does this mean?

It means someone looking through a 'diff' to see what changed, sees a diff on the line:

        if(i % (5*3) == 0) {

and also on the line:

        } else if (....)

when, in fact, nothing in those lines of any consequence changed.....

By not adding the braces when you created the code, you have made the maintainability of the code much more difficult, because each maintenance requires a lot more effort, and a lot more confusion when trying to identify code progression.

What saved you 2 seconds when you wrote the code, is going to create 5 minutes of effort later for every single person who investigates why you changed lines of code, that you did not actually change.... and those 'unchanged' but different lines are the most complicated parts of the code...

By not adding the braces, you are essentially saying, "My 2-seconds saved is worth more than the hours? of time that someone else has to consume understanding revisions of my code).

Now, if that is not enough, whenever work is done, bugs can be introduced, and the amount of bugs is typically proportional to the complication and effort required to implement a change....

By making changes to your code more difficult, you are increasing the amount of bugs that result from your laziness, because other people need to correct both the code, and your laziness in the future.

Note here is the diff for adding in a count of the number of FizzBuzz's using your code:

Now compare that diff with what it looks like if the bracing is right to start with:

Answer 2

Using an IDE's reformat code function (IntelliJ or Eclipse), your program would look like this:

public static void main(String[] args) {
    for (int i = 1; i < 101; i++)
        if (i % (5 * 3) == 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);
}

Notice the whitespaces. I think it's ideal when code looks as close as possible to what an IDE reformats. That way it should look familiar to everyone, and facilitate easier reading. Not everybody shares this opinion. But since you asked about Java coding style, I'm mentioning this.

About not using braces... It's recommended to use the braces. Static analysis tools like Sonar flag these as minor violations. Even if you disagree with that and don't want to use it with simple if statements, you should at least use with the for loop. I think it's better to follow the recommendations of static analysis tools then switching them off.

Other than coding style, the biggest "problem" with this fizzbuzz implementation is the same as with all the most common approaches: it's not very extensible. If you wanted to add "Jazz" with the number 7, you would have to add all permutations. And it gets even worse if you add more divisors.