I had an interview recently where I was asked to produce the traditional FizzBuzz solution:
Output a list of numbers from 1 to 100.
- For all multiples of 3 and 5, the number is replaced with "FizzBuzz"
- For all remaining multiples of 3, the number is replaced with "Fizz"
- For all remaining multiples of 5, the number is replaced with "Buzz"
My solution was written in Java because of the role, but this was not a requirement. The interviewer was keen to see some evidence of TDD, so in that spirit I went about producing a FizzBuzz unit test:
public class FizzBuzzTest {
@Test
public void testReturnsAnArrayOfOneHundred() {
String[] result = FizzBuzz.getResultAsArray();
assertEquals(100, result.length);
}
@Test
public void testPrintsAStringRepresentationOfTheArray() {
String result = FizzBuzz.getResultAsString();
assertNotNull(result);
assertNotSame(0, result.length());
assertEquals("1, 2", result.substring(0, 4));
}
@Test
public void testMultiplesOfThreeAndFivePrintFizzBuzz() {
String[] result = FizzBuzz.getResultAsArray();
// Check all instances of "FizzBuzz" in array
for (int i = 1; i <= 100; i++) {
if ((i % 3) == 0 && (i % 5) == 0) {
assertEquals("FizzBuzz", result[i - 1]);
}
}
}
@Test
public void testMultiplesOfThreeOnlyPrintFizz() {
String[] result = FizzBuzz.getResultAsArray();
// Check all instances of "Fizz" in array
for (int i = 1; i <= 100; i++) {
if ((i % 3) == 0 && !((i % 5) == 0)) {
assertEquals("Fizz", result[i - 1]);
}
}
}
@Test
public void testMultiplesOfFiveOnlyPrintBuzz() {
String[] result = FizzBuzz.getResultAsArray();
// Check all instances of "Buzz" in array
for (int i = 1; i <= 100; i++) {
if ((i % 5) == 0 && !((i % 3) == 0)) {
assertEquals("Buzz", result[i - 1]);
}
}
}
}
My resulting implementation became:
public class FizzBuzz {
private static final int MIN_VALUE = 1;
private static final int MAX_VALUE = 100;
private static String[] generate() {
List<String> items = new ArrayList<String>();
for (int i = MIN_VALUE; i <= MAX_VALUE; i++) {
boolean multipleOfThree = ((i % 3) == 0);
boolean multipleOfFive = ((i % 5) == 0);
if (multipleOfThree && multipleOfFive) {
items.add("FizzBuzz");
}
else if (multipleOfThree) {
items.add("Fizz");
}
else if (multipleOfFive) {
items.add("Buzz");
}
else {
items.add(String.valueOf(i));
}
}
return items.toArray(new String[0]);
}
public static String[] getResultAsArray() {
return generate();
}
public static String getResultAsString() {
String[] result = generate();
String output = "";
if (result.length > 0) {
output = Arrays.toString(result);
// Strip out the brackets from the result
output = output.substring(1, output.length() - 1);
}
return output;
}
public static final void main(String[] args) {
System.out.println(getResultAsString());
}
}
Self-examining what I originally submitted:
Early on I decided to merge my "multiple of" calculation into the generate() method to avoid over-engineering, which I now think was a mistake
The separate getResultAsArray/generate methods were clearly OTT.
The getResultAsString could also be merged with the main() method, since one just delegates to the other.
@APC on StackOverflow pointed out that my approach is not scalable, perhaps I should have better separated the logic from the string building?
I'm still fairly inexperienced with TDD and I feel this may have let me down in this case. I'm looking for other ways I might have improved on this approach, particularly with regard to TDD practices?
This is cross-referenced from my SO question, since you can't move SO questions to Code Review, and someone pointed out that this is a better forum for this type of request. In retrospect, I should have come here in the first place!
Based on the very useful suggestions on StackOverflow, I've reworked my answer to something I now consider would have been more "TDD-friendly". This is my second attempt at a solution:
Separated the FizzBuzz logic from the output generation to make the solution more scalable
Just one assertion per test, to simplify them
Only testing the most basic unit of logic in each case
A final test to confirm the string building is also verified
The code:
public class FizzBuzzTest {
@Test
public void testMultipleOfThreeAndFivePrintsFizzBuzz() {
assertEquals("FizzBuzz", FizzBuzz.getResult(15));
}
@Test
public void testMultipleOfThreeOnlyPrintsFizz() {
assertEquals("Fizz", FizzBuzz.getResult(93));
}
@Test
public void testMultipleOfFiveOnlyPrintsBuzz() {
assertEquals("Buzz", FizzBuzz.getResult(10));
}
@Test
public void testInputOfEightPrintsTheNumber() {
assertEquals("8", FizzBuzz.getResult(8));
}
@Test
public void testOutputOfProgramIsANonEmptyString() {
String out = FizzBuzz.buildOutput();
assertNotNull(out);
assertNotSame(0, out.length());
}
}
public class FizzBuzz {
private static final int MIN_VALUE = 1;
private static final int MAX_VALUE = 100;
public static String getResult(int input) {
boolean multipleOfThree = ((input % 3) == 0);
boolean multipleOfFive = ((input % 5) == 0);
if (multipleOfThree && multipleOfFive) {
return "FizzBuzz";
}
else if (multipleOfThree) {
return "Fizz";
}
else if (multipleOfFive) {
return "Buzz";
}
return String.valueOf(input);
}
public static String buildOutput() {
StringBuilder output = new StringBuilder();
for (int i = MIN_VALUE; i <= MAX_VALUE; i++) {
output.append(getResult(i));
if (i < MAX_VALUE) {
output.append(", ");
}
}
return output.toString();
}
public static final void main(String[] args) {
System.out.println(buildOutput());
}
}
