2
\$\begingroup\$

I was trying to implement FizzBuzz without modulus operator.

Areas of concern: inner for loop maybe not needed? seq[] array maybe not needed?

class FizzBuzzWithoutModulus{
    public static void main(String[] args){

    int[] fizzbuzz = new int[101];
    int[] buzz = new int[101];
    int[] fizz = new int[101];

    int[] seq = new int[101];
    for(int i=1; i<=100; i++){
      seq[i] = i;
    }

    for(int i=1; i<=100; i++){
      for(int j=1; j<seq.length; j++){
        if((i/15.0) == seq[j]){
          fizzbuzz[i] = i;
        }
        else if((i/5.0) == seq[j]){
          buzz[i] = i;
        }
        else if((i/3.0) == seq[j]){
          fizz[i] = i;
        }
      }
    }
    for(int i=0; i<=100; i++){
      if(fizzbuzz[i]!=0){
        System.out.println(fizzbuzz[i] + " fizzbuzz");
      }
      else if(buzz[i]!=0){
        System.out.println(buzz[i] + " buzz");
      }
      else if(fizz[i]!=0){
        System.out.println(fizz[i] + " fizz");
      }
    }
  }
}
\$\endgroup\$
  • \$\begingroup\$ Why are you dividing by a float instead of an integer? \$\endgroup\$ – Mast May 12 '18 at 19:51
  • \$\begingroup\$ @Mast float literal is needed to avoid Java automatically truncate the int. Otherwise 47/15 for example will evaluate to 3, erroneously flagging 47 as fizzbuzz. \$\endgroup\$ – BridgeWall May 12 '18 at 20:26
  • \$\begingroup\$ Accessing an element of an array is \$O(1)\$ and probably pretty fast, so the calls to seq[j] should not be a bottleneck. However, the initial filling up of seq takes time, which you could save if you replaced the array seq with a method int seq(int n) {return n;}. \$\endgroup\$ – Stingy May 12 '18 at 21:36
3
\$\begingroup\$

I think this implementation is really too confusion for the problem at hand, although I think it is quite an interesting solution.

As you said it yourself, integer division loses the fractional part (rounds down towards 0). You can actually use this for your solution! Because if a number is not evenly divisible, multiplying with the division result will not give you back the original number, it will be smaller!

So a way to do this would be:

public static String fizzBuzz(final int number) {
    if (number / 15 * 15 == number) {
        return "FizzBuzz";
    } else if (number / 5 * 5 == number) {
        return "Buzz";
    } else if (number / 3 * 3 == number) {
        return "Fizz";
    }
    return Integer.toString(i);
}

Note that a number divisible by 3 and 5 is also divisible by 15.

\$\endgroup\$
1
\$\begingroup\$

I think there is an error because in fizzbuzz you usually just print out the raw number if it's not a multiple of 3 or 5.

I don't like hard coded "magic numbers". It's better to define a constant final int n = 100 and use that everywhere. If you change it later, you won't have to worry that maybe you missed an instance. An alternative would be to write this as a function and have one input parameter n.

Instead of the inner loop, using a binary search would be a lot more efficient.

I would probably rename seq to integers. Also, seq does not have to go to 100, but could stop at roughly 100 / 3.

Whenever you check if the divided number is an integer, eg. (i/15.0) == seq[j], I would probably write that as a function checkIfInteger(i / 15.0). That function could have the same implementation that you have with seq, or any other implementation. This is the principle of "separation of concerns": you basically break up your code in smaller parts so it's easier to read and maintain.

This type of problem is a natural fit with modern Streams, which are a more readable alternative to for-loops in some cases. For example, for the standard fizzbuzz algo (copied from @Ronald Raab's answer here):

IntStream.rangeClosed(0, 100).mapToObj(
    i -> i % 3 == 0 ?
            (i % 5 == 0 ? "FizzBuzz" : "Fizz") :
            (i % 5 == 0 ? "Buzz" : i))
    .forEach(System.out::println);
\$\endgroup\$
0
\$\begingroup\$

One option you have is to use your own mod method. If you subtract the answer after the div and multiply you get a modulus the same as the modulus operator, which can go in a method.

By using floating indexes for the whole combined string, you can simplify by only checking for each divisor and returning however much of the string is needed. Something like this:

public static int mod(int num, int divisor)
{
    return num - ((num / divisor) * divisor);
}

public static String fizzBuzz(int num, int divisor1, int divisor2)
{
    final String answer = "FizzBuzz";
    final int firstStart = 0;
    final int firstEnd = 4;
    final int secondStart = 4;
    final int secondEnd = 8;
    int start = firstStart;
    int end = start;
    if (mod(num, divisor1) == 0)
    {
        end = firstEnd;
    }
    if (mod(num, divisor2) == 0)
    {
        if (end == 0)
        {
            start = secondStart;
        }
        end = secondEnd;
    }
    if (end > 0)
    {
        return answer.substring(start, end);
    }
    return Integer.toString(num);
}

The ends are set to 1 past the actual end to simplify calling substring

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.