FizzBuzz implementation

  • The common FizzBuzz implementation I saw is using a check for % 15 for printing "FizzBuzz"



    Would you let me know if there is anything wrong / better with this approach?



    public class FizzBuzz {
    public static void main(String[] args) {
    for (int i = 1; i <= 100; i++) {
    boolean fizzOrBuzz = false;
    if (i % 3 == 0) {
    System.out.print("Fizz");
    fizzOrBuzz = true;
    }
    if (i % 5 == 0) {
    System.out.print("Buzz");
    fizzOrBuzz = true;
    }

    if (fizzOrBuzz) {
    System.out.println();
    } else {
    System.out.println(String.valueOf(i));
    }
    }
    }
    }

    Your version is optimized for the computer. Winston's version is optimized for the human. Got to trust a compiler that it can do a good job at re-shuffling and optimizing the code. Also, it is possible that a single io statement will work faster than two separate ones. You should race your version against the other one.

    I'll tell you why I like my version better (only for the interview context) 1) maintainability - you can externalize Fizz and Buzz as strings, you don't need to externalize FizzBuzz 2) DRY, a user writing the above code is thinking in terms of not repeating oneself, which I like

    +1 but, use a *StringBuilder* and not 100 * System.out.println(..), and, *Integer.valueOf(i)*

    @cl-r thanks, I will keep it at the original so the comments will still make sense, thanks for the feedback

    FizzBuzzEnterpriseEdition is the best implementation there is: https://github.com/EnterpriseQualityCoding/FizzBuzzEnterpriseEdition ;D

  • Let's compare your version to the % 15 version:



    public class FizzBuzz {
    public static void main(String[] args) {
    for (int i = 1; i <= 100; i++) {
    if (i % 15 == 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(String.valueOf(i));
    }
    }
    }
    }


    The % 15 version is simpler and easier to read. This version neatly delineates the problem into the 4 different cases, and handles each case. In contrast, your version introduces a boolean logic flag (which I consider to be a significant anti-pattern) and a not entirely intuitive dependence on the order of the if statements.


    15 is a "magic number" here. I'd suggest using `(3 * 5)` instead. The compiler should convert it to 15...

    @Ron - So is, `"FizzBuzz"`, by this criteria. Not that you're _wrong_ necessarily, but for these types of 'trivia' questions, the necessity of declaring everything is usually relaxed slightly. For instance, strictly speaking, for production-level code, **all** the numbers and strings referenced/printed here should be captured as named constants.

    @X-Zero, would you capture the 3 as a constant, what would you call it?

    @Winston - The general problem with this is it's basically a 'contrived' example, so it can be hard to get _good_ names for this. Especially as what the test has is essentially 'multiple' checks - continuing to decompose this would likely result in some sort of array/table structure (yeah... I know... heading to outer-space architecture). So, `MATCH` maybe? `CONDITION`? Urgh. Which is why these kinds of conventions are relaxed for these kinds of tests; the test is for basic coding ability, not (necessarily) full abstraction skills.

    I would move if-for-15 inside if-for-5, if you can't divide a number by 3, certainly can't divide by 15, save 4/5 of checks for all numbers

  • It looks fine. String.valueOf() is unnecessary, System.out.println(i) would print the same but it is still OK. This test is used only to make sure that the interviewee can write code as the linked site says:




    This sort of question won’t identify great programmers, but it will identify the weak ones.
    And that’s definitely a step in the right direction.



  • Here's a version that's (IMHO) a little better for humans than yours and better for computers as well:



    public class FizzBuzz {
    public static void main(String[] args) {
    for (int i = 1; i <= 100; i++) {
    String value;
    switch (i % 15) {
    case 3:
    case 6:
    case 9:
    case 12: // divisible by 3, print Fizz
    value = "Fizz";
    break;
    case 5:
    case 10: // divisible by 5, print Buzz
    value = "Buzz";
    break;
    case 0: // divisible by 3 and by 5, print FizzBuzz
    value = "FizzBuzz";
    break;
    default: // else print the number
    value = Integer.toString(i);
    }
    System.out.println(value);
    }
    }
    }


    The comments provide information to humans (but they could still see it on their own) and there's only one System.out.println call per i.



    EDIT: This is another way to fizz-buzz (focus: DRY):



    public class FizzBuzz {
    public static void main(String[] args) {
    final String EMPTY = "";
    for (int i = 1; i <= 100; i++) {
    String value = EMPTY;
    if (i % 3 == 0) value += "Fizz";
    if (i % 5 == 0) value += "Buzz";
    if (value == EMPTY) value += i;
    System.out.println(value);
    }
    }
    }


    EDIT 2: yet another, using StringBuilder, DRY as well:



    public class FizzBuzz {
    public static void main(String[] args) {
    StringBuilder builder = new StringBuilder(1000);
    for (int i = 1; i <= 100; i++) {
    final int length = builder.length();
    if (i % 3 == 0) builder.append("Fizz");
    if (i % 5 == 0) builder.append("Buzz");
    if (length == builder.length()) builder.append(i);
    builder.append('\n');
    }
    System.out.println(builder);
    }
    }

    StringBuilder seems a faster approach than += in this case

    @EranMedan - Are you sure of this? `StringBuilder` is good because `N` appends doesn't lead to `N` allocations, however here `N` is at most 2, which is a small enough number that it likely won't make a difference, and it's quite likely to be 1, which means no difference at all (or maybe worse, as `StringBuilder` probably does an extra allocation for that 1 append).

    @asveikau - you are probably right, I guess this will be over optimization to find out exactly which is faster :)

    I'm aware of StringBuilder - which I use all the time in String heavy code - and thought along the lines of asveikau. `+=` is less "noisy" (also the reason for my avoidance of `Integer.toString()`) and it won't make much of a difference as there's at most two concatenations. Btw.: I'd have used StringBuilder like this: declare before the loop, initialize with capacity set to `8`, cleared and reuse per iteration. Creating it inside the loop wouldn't improve much.

    see above for another version using `StringBuilder` - but only one call to `System.out.println(...)`.

    if edit 1 you dont have to do += here right? - if (value == EMPTY) value += i;

  • They are certainly not perfects, but I've tried some ways to optimize the test, here the result (I have had numbers to keep trace of good responses) , and I use a StringBuilder to avoid initialization of output IO:



    package exercices;

    import java.util.Hashtable;

    import org.memneuroo.outils.communs.utilitaires.EnvPrm;

    public class FizzBuzz {
    // time for cum with nbIter=30 > 300; 30 ~= 3000
    static final int nbIter = 30;
    static final String sep = "_";

    static long ifNested() {
    final StringBuilder sb = new StringBuilder();
    final long t = System.nanoTime();
    for (int i = 0; i < nbIter; i++) {
    sb.append(//
    i % 15 == 0 //
    ? "FizzBuzz" //
    : (i % 3 == 0 //
    ? "Fizz"//
    : (i % 5 == 0//
    ? "Buzz" //
    : i)));// sb.append(sep);
    }
    final long totT = System.nanoTime() - t;
    System.out.format("ifNested\t%20d\n", totT);
    // sb.append(EnvPrm.NEWLINE); System.out.println(sb.toString());
    return totT;
    }

    static long stringPlus() {
    final StringBuilder sb = new StringBuilder();
    final long t = System.nanoTime();
    for (int i = 0; i < nbIter; i++) {
    String x = "";
    x += (i % 3 == 0) ? "Fizz" : "";
    x += (i % 5 == 0) ? "Buzz" : "";
    if (x.isEmpty()) { // MODIF
    x += Integer.toString(i);
    }
    sb.append(x);// sb.append(sep);
    }
    final long totT = System.nanoTime() - t;
    System.out.format("stringPlus\t%20d\n", totT);
    // sb.append(EnvPrm.NEWLINE); System.out.println(sb.toString());
    return totT;
    }

    static long withIf() {
    final StringBuilder sb = new StringBuilder();
    final long t = System.nanoTime();
    for (int i = 0; i < nbIter; i++) {
    if (i % 3 == 0) {
    sb.append("Fizz");
    if (i % 5 == 0) {
    sb.append("Buzz");
    }
    } else if (i % 5 == 0) {
    sb.append("Buzz");
    } else {
    sb.append(i);
    }// sb.append(sep);
    }
    final long totT = System.nanoTime() - t;
    System.out.format("withIf\t\t%20d\n", totT);
    // sb.append(EnvPrm.NEWLINE);System.out.println(sb.toString());
    return totT;
    }

    static long withArray() {
    final String[] lis = {"FizzBuzz", "", "", "Fizz", "", "Buzz", "Fizz",
    "", "", "Fizz", "Buzz", "", "Fizz", "", "",};
    final StringBuilder sb = new StringBuilder();
    final long t = System.nanoTime();
    for (int i = 0; i < nbIter; i++) {
    final String pos = lis[i % 15];
    sb.append(((0 == pos.length()) ? i : pos));// sb.append(sep);
    }
    final long totT = System.nanoTime() - t;
    System.out.format("withArray\t%20d\n", totT);
    // sb.append(EnvPrm.NEWLINE); System.out.println(sb.toString());
    return totT;
    }

    static long withTable() {
    final Hashtable<Integer, String> ht = new Hashtable<>(8);
    ht.put(0, "FizzBuzz");
    ht.put(3, "Fizz");
    ht.put(5, "Buzz");
    ht.put(6, "Fizz");
    ht.put(9, "Fizz");
    ht.put(10, "Buzz");
    ht.put(12, "Buzz");
    final StringBuilder sb = new StringBuilder();
    final long t = System.nanoTime();
    for (int i = 0; i < nbIter; i++) {
    final String s = ht.get(i % 15);
    // MODIF
    // http://www.developpez.net/forums/d1196563-2/java/general-java/if-null-object-if-objet-null/#post6561766
    // sb.append((null == s ? i : s));// sb.append(sep);
    if (null == s) {
    sb.append(i);
    } else {
    sb.append(s);
    }
    }
    final long totT = System.nanoTime() - t;
    System.out.format("withTable\t%20d\n", totT);
    // sb.append(EnvPrm.NEWLINE); System.out.println(sb.toString());
    return totT;

    }

    static int recursive(final StringBuilder sb, final int n) {
    if (0 == n) {
    return 1;
    }
    if (n % 3 == 0) {
    sb.insert(0, "Fizz");
    if (n % 5 == 0) {
    sb.insert(0, "Buzz");
    }
    } else if (n % 5 == 0) {
    sb.insert(0, "Buzz");
    } else {
    sb.insert(0, n);
    }
    return n + recursive(sb, n - 1);
    }

    static long recursive() {
    final StringBuilder sb = new StringBuilder("");
    final long t = System.nanoTime();
    recursive(sb, nbIter);
    final long totT = System.nanoTime() - t;
    System.out.format("recursive\t%20d\n", totT);
    sb.append(EnvPrm.NEWLINE);
    System.out.println(sb.toString());
    return totT;
    }

    /*** @param args */
    public static void main(final String[] args) {
    long cum = 0L, cum2 = 0L;
    for (int i = 0; i < 5; i++) {
    System.out.println("------ " + i + " -----");
    final long totSb = stringPlus();
    final long totIn = ifNested();
    final long totWi = withIf();
    final long totWa = withArray();
    final long totWt = withTable();
    final long totRe = recursive();
    System.out.format("... stringPlus/withIf :%5d\n", (totSb * 100)
    / totWi);
    System.out.format("... ifNested/withIf :%5d\n", (totIn * 100)
    / totWi);
    System.out.format("... withArray/withIf :%5d\n", (totWa * 100)
    / totWi);
    System.out.format("... withTable/withIf :%5d\n", (totWt * 100)
    / totWi);
    System.out.format("... recursive/withIf :%5d\n", (totRe * 100)
    / totWi);
    cum += totIn + totSb + totWi + totWa + totWt + totRe;
    System.out.println("CUMUL (SECOND) == " + cum / 100000000 + "."
    + cum % 100000000 + "\t , diff: " + (cum - cum2));
    cum2 = cum;
    }
    }
    }


    And the output :



    ------ 0 -----
    stringPlus 529397
    ifNested 643657
    withIf 27657
    withArray 43581
    withTable 40788
    recursive 87441
    12Fizz4BuzzFizz78FizzBuzz11Fizz1314BuzzFizz1617Fizz19BuzzFizz2223FizzBuzz26Fizz2829BuzzFizz

    ... stringPlus/withIf : 1914
    ... ifNested/withIf : 2327
    ... withArray/withIf : 157
    ... withTable/withIf : 147
    ... recursive/withIf : 316
    CUMUL (SECOND) == 0.1372521 , diff: 1372521
    ------ 1 -----
    stringPlus 345295
    ifNested 88280
    withIf 88279
    withArray 88838
    withTable 101689
    recursive 93308
    12Fizz4BuzzFizz78FizzBuzz11Fizz1314BuzzFizz1617Fizz19BuzzFizz2223FizzBuzz26Fizz2829BuzzFizz

    ... stringPlus/withIf : 391
    ... ifNested/withIf : 100
    ... withArray/withIf : 100
    ... withTable/withIf : 115
    ... recursive/withIf : 105
    CUMUL (SECOND) == 0.2178210 , diff: 805689
    ------ 2 -----
    stringPlus 380216
    ifNested 36597
    withIf 20953
    withArray 60063
    withTable 91352
    recursive 111467
    12Fizz4BuzzFizz78FizzBuzz11Fizz1314BuzzFizz1617Fizz19BuzzFizz2223FizzBuzz26Fizz2829BuzzFizz

    ... stringPlus/withIf : 1814
    ... ifNested/withIf : 174
    ... withArray/withIf : 286
    ... withTable/withIf : 435
    ... recursive/withIf : 531
    CUMUL (SECOND) == 0.2878858 , diff: 700648
    ------ 3 -----
    stringPlus 489168
    ifNested 29613
    withIf 22070
    withArray 27099
    withTable 27378
    recursive 91911
    12Fizz4BuzzFizz78FizzBuzz11Fizz1314BuzzFizz1617Fizz19BuzzFizz2223FizzBuzz26Fizz2829BuzzFizz

    ... stringPlus/withIf : 2216
    ... ifNested/withIf : 134
    ... withArray/withIf : 122
    ... withTable/withIf : 124
    ... recursive/withIf : 416
    CUMUL (SECOND) == 0.3566097 , diff: 687239
    ------ 4 -----
    stringPlus 143035
    ifNested 24025
    withIf 15924
    withArray 23187
    withTable 26819
    recursive 87162
    12Fizz4BuzzFizz78FizzBuzz11Fizz1314BuzzFizz1617Fizz19BuzzFizz2223FizzBuzz26Fizz2829BuzzFizz

    ... stringPlus/withIf : 898
    ... ifNested/withIf : 150
    ... withArray/withIf : 145
    ... withTable/withIf : 168
    ... recursive/withIf : 547
    CUMUL (SECOND) == 0.3886249 , diff: 320152

    StringBuilder is based on a `char16 works and the issues it has. And I'd advise you to do a lot more iterations. I think you mainly measure the time for `System.out.println()` right now, the other operations probably take very little time compared to a system call.

    Tell me, have you ever heard of the concept *over-engineering*...?

    @Max I do, but is this site is done for some researches or not? Is for comfortable eyes, or to try to understand how some ways are better ? So you can choose and compare the faster, the easy to read, or interrogate the code to find missing optimization.

    See Rules for optimization. IMHO Winston Ewert provided the best answer.

    explain math in withTable code please?

    @KalpeshSoni - It is not a math answer, but Java code sample, no abstraction but algorithm application

    never mine I read the other answer by @arne and understood

License under CC-BY-SA with attribution


Content dated before 7/24/2021 11:53 AM