How to reduce if statements

Question

The program below functions as necessary but how do I reduce the amount of if statements. I have been told that if your function contains 2 or more if statements then your doing it wrong. Any suggestions? I've tried using switch statements but that hasn't worked because the case can't be a boolean.

for(int i = 1; i < 100; i++)
        {
        if(i % 10 == 3) 
        {
            System.out.println("Fizz" + "(" + i + ") 3%10");
        }

        if(i / 10 == 3)
        {
            System.out.println("Fizz" + "(" + i + ") 3/10");
        }


        if(i % 10 == 5) 
        {
            System.out.println("Buzz" + "(" + i + ") 5%10");
        }

        if(i / 10 == 5)
        {
            System.out.println("Fizz" + "(" + i + ") 5/10");
        }

        if(i / 10 == 7)
        {
            System.out.println("Fizz" + "(" + i + ") 7/10");
        }

        if(i%10 == 7)
        {
            System.out.println("Woof" + "(" + i + ") 7%10");
        }

        if(i % 3 == 0)
        {
            System.out.println("Fizz" + "(" + i + ") 3%==0");
        }

        if(i % 5 == 0)
        {
            System.out.println("Buzz" + "(" + i + ")5%==0");
        }

        if(i % 7 == 0)
        {
            System.out.println("Woof" + "(" + i + ")7%==0");    
        }

        if( (i % 7 !=0 ) && (i % 3 !=0 ) && (i % 5 !=0 )
                && (i % 10 !=3) && (i % 10 !=5 ) && (i%10 !=7 ) )
            System.out.println(i);
    }

Solution 1

How about creating a method for the cases:

 public void printIfMod(int value, int mod){
       if (value % 10 == mod)
          System.out.println(...);
 }

 public void printIfDiv(int value, int div){
       if (value / 10 == div)
          System.out.println(...);
 }

Then instead of a bunch of if you have a set of calls the the two methods. You might even create a single method that calls both of the above.

 public void printIf(int value, int div){
      printIfMod(value, div);
      printIfDiv(value, div);
 }

 for(int i = 1; i < 100; i++) {
      printIf(i, 3);
      printIf(i, 5);
      ....
 }

In the above code the number of ifs is less of an issue to me than the amount of repeated code.

Solution 2

Here's a slight improvement using two switch statements

switch(i / 10){
  case 3: // do something
    break;
  case 5: // do something else
    break;
  case 7: // do something else
    break;
}

switch(i % 10){
  case 3: // do something
    break;
  case 5: // do something else
    break;
  case 7: // do something else
    break;
}

Unfortunately you'll need one switch statement per divisor.

Alternatively, you can embrace OOP and come up with an abstraction like this:

public abstract class Processor {
    private final int divisor;
    private final int result;
    private final boolean useDiv; // if true, use /, else use %

    public Processor(int divisor, int result, boolean useDiv) {
        this.divisor = divisor;
        this.result = result;
        this.useDiv = useDiv;
    }
    public final void process(int i){
        if (
             (useDiv && i / divisor == result)
             || (!useDiv && i % divisor == result)
           ){
                doProcess(i);
            }
    }

    protected abstract void doProcess(int i);
}

Sample usage:

public static void main(String[] args) {
    List<Processor> processors = new ArrayList<>();
    processors.add(new Processor(10, 3, false) {
        @Override
        protected void doProcess(int i) {
            System.out.println("Fizz" + "(" + i + ") 3%10");
        }
    });
    // add more processors here
    for(int i = 1; i < 100; i++){
        for (Processor processor : processors) {
            processor.process(i);
        }
    }

}

Solution 3

Generally speaking, it's true that code that has a lot of if statements looks suspicious. Suspicious doesn't necessarily mean wrong. If the problem statement has disjoint conditions to check for (i.e. you can't group them) then you have to do them independently like you're doing.

In your case, you're having to check for divisibility without being able to infer one from the other (i.e. if x is divisible by 7, it doesn't mean it's also divisible by 5, etc...). All the numbers you are using have been purposely chosen prime so this is why you're getting into this.

If for example they had said, check for divisibility by 2, 3, and 6. Then you could first check for 6 because then you could also imply divisibility by 2 and 3. Or vice-versa, check by 2 and 3 and imply that it's also divisible by 6. If all the numbers are prime, then you just can't imply. So you're code has to check for everything individually.

One positive side effect is it makes your intent easy to read in your code (because it's all explicit).

My two cents on this...

Solution 4

Enums are a good fit here. They allow you to encapsulate the functionality in one location rather than spreading it throughout your flow control.

public class Test {
  public enum FizzBuzz {
    Fizz {
      @Override
      String doIt(int n) {
        return (n % 10) == 3 ? "3%10"
                : (n / 10) == 3 ? "3/10"
                : (n / 10) == 5 ? "5/10"
                : (n / 10) == 7 ? "7/10"
                : (n % 3) == 0 ? "3%==0"
                : null;
      }

    },
    Buzz {
      @Override
      String doIt(int n) {
        return (n % 10) == 5 ? "5%10"
                : (n % 5) == 0 ? "5%==0"
                : (n / 10) == 3 ? "3/10"
                : (n / 10) == 5 ? "5/10"
                : (n / 10) == 7 ? "7/10"
                : null;
      }

    },
    Woof {
      @Override
      String doIt(int n) {
        return (n % 10) == 7 ? "7%10"
                : (n % 7) == 0 ? "7%==0"
                : null;
      }

    };

    // Returns a String if this one is appropriate for this n.
    abstract String doIt(int n);

  }

  public void test() {
    // Duplicates the posters output.
    for (int i = 1; i < 100; i++) {
      boolean doneIt = false;
      for (FizzBuzz fb : FizzBuzz.values()) {
        String s = fb.doIt(i);
        if (s != null) {
          System.out.println(fb + "(" + i + ") " + s);
          doneIt = true;
        }
      }
      if (!doneIt) {
        System.out.println(i);
      }
    }
    // Implements the game.
    for (int i = 1; i < 100; i++) {
      boolean doneIt = false;
      for (FizzBuzz fb : FizzBuzz.values()) {
        String s = fb.doIt(i);
        if (s != null) {
          if ( doneIt ) {
            System.out.print("-");
          }
          System.out.print(fb);
          doneIt = true;
        }
      }
      if (!doneIt) {
        System.out.print(i);
      }
      System.out.println();
    }
  }

  public static void main(String args[]) {
    try {
      new Test().test();
    } catch (Throwable t) {
      t.printStackTrace(System.err);
    }
  }

}

Solution 5

I would argue, that you're asking the wrong question. The question I think you should ask is: "How can I rewrite this code so that it is more easily understood by a human?"

The creed "eliminate if statements" is a general idea to accomplish this, but it depends heavily on the context.

The sad fact is that many of the answers obfuscate this very simple algorithm in the guise of "making it simpler." Never introduce an object to eliminate a couple of if statements. In my work, most code is maintained by people that understand far less about the architecture, math and code than the original author, so to introduce additional constructs and complexity to reduce the code from 50 physical lines to 30 physical lines, but makes it 4 times more difficult to understand is not a win.

View more solutions