HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Multiple FizzBuzz with input from file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filewithfizzbuzzinputmultiplefrom

Problem

I made a fizzbuzz program in Java 7 I'd like reviewed.

Given an input file with three numbers in each line, fizz, buzz, and count:

3 5 10
2 7 15


The output, for each line, should be the numbers from 1 to count, except that multiples of fizz are replaced with "F", multiples of buzz are replaced by "B", and multiples of both with "FB":

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


The code, in Java 7:

```
package com.codeeval.fizzbuzz;

import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;

public class Main {
public static void main(String[] args) throws IOException {
String path = args[0];
List lines = getFileLines(path);

StringBuilder sb = new StringBuilder();
for (String line : lines){
int[] arguments = getFizzBuzzArguments(line);
int x = arguments[0];
int y = arguments[1];
int n = arguments[2];

String result = fizzBuzz(x, y, n);
sb.append(result).append('\n');
}

System.out.println(sb);
}

public static String fizzBuzz(int x, int y, int n) {
StringBuilder sb = new StringBuilder();
for (int i = 1; i getFileLines(String path) throws IOException {
File file = new File(path);
BufferedReader buffer = new BufferedReader(new FileReader(file));
List lines = new ArrayList<>();
String line;
while ((line = buffer.readLine()) != null) {
line = line.trim();
lines.add(line);
}

return lines;
}

private static int[] getFizzBuzzArguments(String line){
String[] arguments = line.split(" ");
return getArrayOfInts(arguments);
}

private static int[] getArrayOfInts(String[] strings){
int[] ints = new int[strings.length];

for (int i = 0; i < strings.length; i++

Solution

Your code is well structured and quite easy to read and understand: this is a very good point about it.

However, there are some things that may be improved, but very few of them seem to be really critical.

main(arg)

Variables int x, int y and int n are treated separately, but indeed they are semantically related, because they describe a "FizzBuzz" to produce. A dedicated class may be created, for example:

public class FizzBuzz {

  private final int x;
  private final int y;
  private final int n;

  public FizzBuzz(int x, int y, int n) {
    this.x = x;
    this.y = y;
    this.n = n;
  }
  // getters, if needed
}


Moreover, your fizzBuzz method may then go directly into this class, named something like processInput(). The loop in main() would look like this:

for (String line : lines) {
  int[] arguments = getFizzBuzzArguments(line);
  FizzBuzz fb = new FizzBuzz(arguments[0], arguments[1], arguments[2]);
  sb.append(fb.processInput()).append('\n');
}


FizzBuzz may also have a constructor taking directly the array of input data as argument.

fizzBuzz(args)

Three points concerning the modulo divisions and the booleans isX and isY.

First, the divisors (x or y) must be validated to be different from 0, if you want to avoid ArithmeticException .

Second, they should be extracted into a dedicated method, which is easier to read:

private static boolean isMultipleOf(int dividend, int divisor) {
  return dividend % divisor == 0;
}


Third, the booleans are named poorly. They'd better sound as isFizz and isBuzz: these are their meanings.

Finally, the conditionals can be simplified:

if (isFizz || isBuzz) {
  if (isFizz) {
    sb.append("F");
  }
  if (isBuzz) {
    sb.append("B");
  }
} else {
  sb.append(i);
}


getFileLines()

Since you underline that the code in Java 7, then its facilities should be used.

-
Replace File with java.nio.file.Path.

-
Use a try-with-resources block in order to close automatically the streams opened with BufferedReader and FileReader.

Code Snippets

public class FizzBuzz {

  private final int x;
  private final int y;
  private final int n;

  public FizzBuzz(int x, int y, int n) {
    this.x = x;
    this.y = y;
    this.n = n;
  }
  // getters, if needed
}
for (String line : lines) {
  int[] arguments = getFizzBuzzArguments(line);
  FizzBuzz fb = new FizzBuzz(arguments[0], arguments[1], arguments[2]);
  sb.append(fb.processInput()).append('\n');
}
private static boolean isMultipleOf(int dividend, int divisor) {
  return dividend % divisor == 0;
}
if (isFizz || isBuzz) {
  if (isFizz) {
    sb.append("F");
  }
  if (isBuzz) {
    sb.append("B");
  }
} else {
  sb.append(i);
}

Context

StackExchange Code Review Q#110745, answer score: 2

Revisions (0)

No revisions yet.