This escalated quickly. And wow, I've never been called enterprisy before, I'm proud. So, anyway, I'm obviously not recommending anyone to do FizzBuzz like this: I mean, it's obviously an overkill, and a horrible first solution to turn in when asked at an interview. I used the basic problem as a springboard to show OP some concepts (magic constants, encapsulation, builder pattern) and also some language features (constructor delegation, Javadoc) they may not be familiar with. Also, as an interviewer I'd bloody well ask follow-up questions like "but I like my FizzBuzz parametrizable" and "what's wrong with this constructor?", and I'd prefer to get answers like this.
And in addition to the answers posted by @Heslacher and @TimothyTruckle: I would take this even further.
Because, really, what are those magic numbers doing in the code? 3, 5 and 100, all code checkers (checkstyle or PMD) would complain about these. Sure, we could extract them into static constants:
private static final int FIZZ_NUMBER = 3
private static final int BUZZ_NUMBER = 5
private static final int ITERATIONS = 100
But I like my FizzBuzz configurable: the user of our FizzBuzz lib should be able to specify these constants. Likewise, just printing stuff on the console is bad form in any language: why can't I supply my own outputstream?
The obvious solution is to encapsulate the FizzBuzz logic into its own class:
public class FizzBuzz {
private final int fizzNumber;
private final int buzzNumber;
private final int iterations;
/**
* Constructor.
*
* @param fizzNumber We should print a "fizz" when the current
* iteration is divisible by this number
* @param buzzNumber We should print a "buzz" when the current
* iteration is divisible by this number
* @param iterations The simulation should run this many cycles.
*/
public FizzBuzz(int fizzNumber, int buzzNumber, int iterations) {
this.fizzNumber = fizzNumber;
this.buzzNumber = buzzNumber;
this.iterations = iterations;
}
/**
* Constructor creating a vanilla FizzBuzz, with parameters 3, 5, and 100
*/
public FizzBuzz() {
this(3, 5, 100);
}
/**
* Write fizzes and buzzes on the given output stream.
*
* @param out The outputStream we should write fizzes and buzzes on.
*/
public void write(OutputStream out) {
// fizzbuzz logic
}
/**
* Write fizzes and buzzes on the standard output.
*/
public void write() {
this.write(System.out);
}
}
Call it like this:
FizzBuzz writer = new FizzBuzz(5, 2, 200);
writer.write();
That constructor, though... 5 and 2, which was the Fizz and which was the Buzz? Writing constructors with a relatively large numbers of indistinguishable paramters harms the readability of the code. I'm really fond of the builder pattern of Joshua Bloch:
public class FizzBuzz {
private FizzBuzz(int fizzNumber, int buzzNumber, int iterations) {
this.fizzNumber = fizzNumber;
this.buzzNumber = buzzNumber;
this.iterations = iterations;
}
/* FizzBuzz implementation here. */
public Builder create() {
return new Builder();
}
public static final class Builder {
private int fizzNumber = 3;
private int buzzNumber = 5;
private int iterations = 100;
private Builder() {
// private ctor to hide the default one.
}
public Builder withFizzNumberOf(int fizzNumber) {
this.fizzNumber = fizzNumber;
return this;
}
public Builder withBuzzNumberOf(int buzzNumber) {
this.buzzNumber = buzzNumber;
return this;
}
public Builder withIterations(int iterations) {
this.iterations = iterations;
return this;
}
public FizzBuzz done() {
// perform checks on the parameters here.
return new FizzBuzz(fizzNumber, buzzNumber, iterations);
}
}
}
Call it like this:
FizzBuzz fizzBuzz = FizzBuzz.create()
.withFizzNumber(7)
.withBuzzNumber(3)
.withIterations(200)
.done();
Oh and, use Javadoc to document your code!
if..else
if you use aString
variable and concatenate using ternary:String strOut = ""; strOut = ((i % 3) ? "Fizz" : "") + ((i % 5) ? "Buzz" : ""); System.out.println((strOut.length() > 0) ? strOut : i)
. I'm not sure how this stands up to everyone's scrutiny, but it reads cleaner to me than anif..else
. \$\endgroup\$