FizzBuzz from a file

Question

This was found on CodeEval.

Challenge:

Write a program that prints out the final series of numbers where those divisible by X, Y and both are replaced by “F” for fizz, “B” for buzz and “FB” for fizz buzz

Specifications:

Your program should accept a file as its first argument. The file contains multiple separated lines; each line contains 3 numbers that are space delimited. The first number is the first divider (X), the second number is the second divider (Y), and the third number is how far you should count (N). You may assume that the input file is formatted correctly and the numbers are valid positive integers. your output should print out one line per set. Ensure that there are no trailing empty spaces in each line you print.

Sample Input:

3 5 10
2 7 15

Sample Output:

1 2 F 4 B F 7 8 F B
1 F 3 F 5 F B F 9 F 11 F 13 FB 15

My Implementation:

import java.io.File;
import java.io.FileNotFoundException;
import java.util.Scanner;

public class FizzBuzz {
    public static void main(String[] args) throws FileNotFoundException {
        File file = new File(args[0]);
        Scanner fileScanner = new Scanner(file);
        while (fileScanner.hasNextLine()) {
            printFizzBuzz(fileScanner.nextLine());
        }
    }

    public static void printFizzBuzz(String line) {
        int fizz = Integer.parseInt(line.split(" ")[0]);
        int buzz = Integer.parseInt(line.split(" ")[1]);
        int limit = Integer.parseInt(line.split(" ")[2]);

        StringBuilder sb = new StringBuilder();

        for (int i = 1; i <= limit; i++) {
            if (i % fizz == 0) { sb.append("F"); }
            if (i % buzz == 0) { sb.append("B"); }
            else { if (i % fizz != 0) { sb.append(i); } }
            if (i < limit) { sb.append(" "); }
        }
        System.out.println(sb.toString());
    }
}

Answer 1

If you extract the logic that builds the output string to its own method buildFizzBuzz, you'll get something testable:

public static String buildFizzBuzz(int fizz, int buzz, int limit) {
    StringBuilder sb = new StringBuilder();
    // ...
    return sb.toString();
}

public static void printFizzBuzz(String line) {
    int fizz = Integer.parseInt(line.split(" ")[0]);
    int buzz = Integer.parseInt(line.split(" ")[1]);
    int limit = Integer.parseInt(line.split(" ")[2]);

    System.out.println(buildFizzBuzz(fizz, buzz, limit));
}

And you can add some unit tests, for example:

@Test
public void test_3_5_20() {
    assertEquals("1 2 F 4 B F 7 8 F B 11 F 13 14 FB 16 17 F 19 B", buildFizzBuzz(3, 5, 20));
}

@Test
public void test_2_7_15() {
    assertEquals("1 F 3 F 5 F B F 9 F 11 F 13 FB 15", buildFizzBuzz(2, 7, 15));
}

Having these unit test, which pass at the moment, now we can refactor a bit safely, knowing that if anything breaks, the tests will break.

It's not great to have a check on the limit twice in the loop, in the loop condition and also in the loop body. It would be better to eliminate the condition from the body, always appending ' ', and cutting off the space at the end.

It's not great to have a check on i % fizz twice. It would be better to use a boolean to track if fizz or buzz were already appended.

Putting these two points together, the implementation becomes:

public static String buildFizzBuzz(int fizz, int buzz, int limit) {
    StringBuilder sb = new StringBuilder();

    for (int i = 1; i <= limit; i++) {
        boolean shouldAppendNum = true;
        if (i % fizz == 0) {
            sb.append("F");
            shouldAppendNum = false;
        }
        if (i % buzz == 0) {
            sb.append("B");
            shouldAppendNum = false;
        }
        if (shouldAppendNum) {
            sb.append(i);
        }
        sb.append(" ");
    }
    sb.setLength(sb.length() - 1);
    return sb.toString();
}