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);
    }

"if your function contains 2 or more if statements then your doing it wrong" I disagree. Some powerfull languages like Perl don't have a switch-like statement. Don't worry about it. We're talking about clear code. Well, using switch in this case is impossible, because your are not using the same condition. (i % number) and (i/number). — cgalvao1993, May 21 '13 at 12:19
You can't avoid the tests, but maybe refactore to function calls to make the whole block lighter. — PeterMmm, May 21 '13 at 12:20
Tell the interviewer you prefer simple constructs (if's) and not over-engineering (all into object's). Maybe he was waiting for "that not follows Java code-styling conventions". — PeterMmm, May 21 '13 at 12:33
"if your function contains 2 or more if statements then your doing it wrong" Nope. Not true. IF-ELSE proliferation is however not modification-friendly, and thus in the real world, where extensibility and maintainability are often requirements, is not the best design. — Mister Smith, May 21 '13 at 13:02
related, but not dupe: http://stackoverflow.com/q/3786358/342852 — Sean Patrick Floyd, May 21 '13 at 13:28
"if your function contains **2 or more** if statements then your doing it wrong" seems extreme. According to http://en.wikipedia.org/wiki/Cyclomatic_complexity#Limiting_complexity_during_development, there's good evidence for a cyclomatic complexity limit of 10, which would translate to "If you have **4 or more** if statements". — LarsH, May 21 '13 at 14:19
@LarsH I think what he means is, "If your implementation of **this particular function** contains two or more `if` statements, you're doing it wrong. — Daniel, May 21 '13 at 15:53
@Daniel: oh, ok. Maybe he does. That seems odd too, but without more context it's hard to tell. — LarsH, May 21 '13 at 17:19

score 52 · Accepted Answer · answered May 21 '13 at 12:18

52

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.

answered May 21 '13 at 12:18

John B

32,493
6
77
98

You could also iterate in a nested for for values in 1, 3, 5, 7, etc to avoid repeating those – Sebastian Piu May 21 '13 at 12:32
3

In fact, you *just* need to iterate over the divisors and there is no need to extract that into a method, which will have a single call site in that case. – Marko Topolnik May 21 '13 at 12:53
2

You need to add the "sound", as in AmitG's answer. `public void printIfXyz(string sound, int value, int numerator) {...}`. The code in the example isn't cleanly 3=fizz/5=buzz/7=woof. (Whether that's intentional or deliberate is unknown). – WernerCD May 21 '13 at 12:56

Sean Patrick Floyd · Answer 2 · 2013-05-21T12:30:36.530

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);
        }
    }

}

score 12 · Answer 3 · answered May 21 '13 at 12:23

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...

OldCurmudgeon · Answer 4 · 2013-05-21T12:38:53.520

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);
    }
  }

}

+1 While I like the enum approach I really can't stand the nested conditional operators. But this may be personal preference. It's just my rule of thumb to never nest conditional operators. — helpermethod, May 21 '13 at 15:32
@helpermethod - I can understand your preference - coming from a C background it feels quite natural. In `Algol` there were also shortened forms for loops and `case` statements too. — OldCurmudgeon, May 21 '13 at 15:44

score 7 · Answer 5 · answered May 21 '13 at 12:27

I had started to write an answer involving code, but many, many people beat me to it. The one thing I would say which hasn't been mentioned yet is that this particular code metric you're referring to is called cyclomatic complexity and isn't a horrendously bad thing.

In short it refers to the number of different paths that a method could take when it's executed, and while it's quite high in the code snipped that you posted, and there are lots of good tips/solutions to reducing it that have been suggested, personally I would argue that even in it's current form, the code is very readable - which is a bonus. It can be reduced a fair amount and still be readable, but my point is that metrics like that aren't everything, and sometimes it can be simpler to have lots of if statements because it's more readable - and readability reduces the likelihood of making mistakes, and makes debugging much easier

Oh, and I would replace this last section:

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

By using a boolean flag such as replaced = true whenever any of the replacement statements are called, then the above statement collapses to:

if (!replaced)
      System.out.println(i);

+1 for referencing prior work on the subject of conditional complexity. — LarsH, May 21 '13 at 14:16

score 7 · Answer 6 · answered May 21 '13 at 13:51

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.

You Mean - you don't subscribe to "If it was hard to program, it should be hard to understand."? ;-) The sample code being slung around here in the question and responses do lack meaningful names in the variable and constants which would make the function and purpose a little more apparent. — Rawheiser, May 21 '13 at 16:50

Bohemian · Answer 7 · 2013-05-21T13:54:28.263

5

Your code is repetitive. Refactor it using loops for your:

for (int i = 1; i < 100; i++) {
    boolean found = false; // used to avoid the lengthy test for "nothing found"
    for (int j = 3; j <= 7; j += 2) { // loop 3, 5, 7
        if (i % 10 == j) {
            System.out.println("Fizz" + "(" + i + ") "+j+"%10");
            found = true;
        }

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

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

    if (!found) {
        System.out.println(i);
    }
}

edited May 21 '13 at 13:54

answered May 21 '13 at 12:59

Bohemian

412,405
93
575
722

2

@ugoren so it is... fixed. `` I typed that code in on my iPhone `` – Bohemian May 21 '13 at 13:47
Almost there - `j <= 7`. – ugoren May 21 '13 at 13:49
2

@Bohemian 100 respect points for writing code on your iphone – Hello-World May 21 '13 at 14:18
2

strongly disagree. you just made the code harder to read and test for bugs. code length != code quality. – Tom Carchrae May 21 '13 at 14:39
@Tom repetition == bad code and is a standard code quality metric. and smaller code length **does** mean code quality if it's legible. i have no trouble reading this simple code, and I find it *way* easier to read than the original. I stand by this answer as a big improvement. – Bohemian May 21 '13 at 15:46
agree w.r.t. simpler and D.R.Y. - but shorter is not always better. i'd way rather 3 lines of code than something like `while (null != result = doSomething(x = getX(y), y++));`. – Tom Carchrae May 21 '13 at 15:56
@tom note that I said "...if it's legible". Of course one can sometimes condense code to the point of encryption. That's not what I am talking about. As long as it's clear, less code is clearer and there are simply less lines in which bugs can lurk, so IMHO (and experience) terse code is *less* buggy than verbose code. – Bohemian May 21 '13 at 16:02
yep - i think we agree. ;) the general rule i use is not lines of code, but rather 'how does the code map to the logical process' - this makes it easier to read/maintain & extend/prove correct, etc. – Tom Carchrae May 21 '13 at 16:19

score 1 · Answer 8 · answered May 21 '13 at 12:25

1

You can create multiple switch:

switch (i/10) {
     case 3:
        System.out.println("Fizz" + "(" + i + ") 3/10");
        break;

    case 5:
        System.out.println("Fizz" + "(" + i + ") 5/10");
        break;

    case 7:
        System.out.println("Fizz" + "(" + i + ") 7/10");
        break;
    default:
        break;
}

switch (i%10) {
    case 3: 
        System.out.println("Fizz" + "(" + i + ") 3%10");
        break;
    case 5:
        System.out.println("Buzz" + "(" + i + ") 5%10");
        break;
    case 7:
        System.out.println("Woof" + "(" + i + ") 7%10");
        break;
    default:
        break;
}

The other case still have to use if statement.
Oracle added switch statement that used String in Java 7. Maybe boolean switch statement will come later.

answered May 21 '13 at 12:25

DeadlyJesus

1,503
2
12
26

There's no point to a boolean `switch`, it's just an `if`/`else`. – Kevin May 21 '13 at 15:13
By boolean switch I mean: `switch(i)` [...] `case (i%10==3): instructions` [...] `case (i/10==5): instructions`. That not the purpose of a switch but that would save a lot of time. – DeadlyJesus May 21 '13 at 15:42
1

Ah yes. The problem with that, though, is that if `i%10 == 3` and `i/10 == 5` (i.e. 53), he wants both to print, but that `switch` would only hit one, presumably the first. – Kevin May 21 '13 at 15:48

score 1 · Answer 9 · answered May 21 '13 at 12:31

public class Test
{

    public static void main(String[] args)
    {

        final int THREE = 3;
        final int FIVE = 5;
        final int SEVEN=7;
        final int ZERO = 0;

        for (int i = 1; i < 100; i++)
        {
            modOperation("Fizz", i, THREE);

            divideOperation("Fizz", i, THREE);


            modOperation("Fizz", i, FIVE);

            divideOperation("Buzz", i, FIVE);



            modOperation("Woof", i, SEVEN);

            divideOperation("Fizz", i, SEVEN);


            modOperation("Fizz", i, ZERO);

            divideOperation("Fizz", i, ZERO);
        }

    }

    private static void divideOperation(String sound, int i, int j)
    {
        if (i / 10 == j) // you can add/expand one more parameter for 10 and later on 3 in this example.
        {
            System.out.println(sound + "(" + i + ") "+j+"/10");
        }
    }

    private static void modOperation(String sound, int i, int j)
    {
        if (i % 10 == j)
        {
            System.out.println(sound + "(" + i + ") "+j+"%10");
        }
    }
}

So now you have less if

Looks close to the higher rated answer... but not sure I understand the need for "THREE", "FIVE", ... Do you think you'll have to update the meaning of those numbers at some point? — WernerCD, May 21 '13 at 12:50
@WernerCD :-) yes. You are correct. You can pass those values directly. By including your thought with above my answer would be kind of yet another possible solution for OP. — AmitG, May 21 '13 at 13:14
ZERO..SEVEN makes sense in real code, since the numbers will have actually meaning, for example VAN_CAPACITY and CAR_CAPACTY in a vehicle scheduling app. Magic numbers, right? This is the best solution to me as it is readable, contains no {...} to figure out as the accepted answer does. I've gotta say, I recently learned of the "IF considered harmful" and trying to grok it, and work it into my code. — David Lotts, Apr 23 '22 at 03:10

How to reduce if statements

9 Answers9

Linked