views:

95

answers:

4

Hi,

I have the following method:

private static final int OPEN_FOR_RAISE_RANGE = 35;
private static final int OPEN_FOR_CALL_RANGE = 25;

public void foo(double num){
    if(num <= OPEN_FOR_RAISE_RANGE){
        //do something
    }
    else if(num <= (OPEN_FOR_RAISE_RANGE + OPEN_FOR_CALL_RANGE)){
        //do something else 
    }
    //etc
}

which basically checks what range the number falls in and acts appropriately. I was wondering if there is a nicer/more efficient way to do this other than lots of if's?

Thanks

+1  A: 

That's about as good as it gets I think.

Note the compiler will replace the OPEN_FOR_RAISE_RANGE + OPEN_FOR_CALL_RANGE calculation with the value 60 at compile time so you're not computing this on every call.

Paolo
+1  A: 

Seems fine by me, it'd be more worrysome if the if-statements were nested like this:

if (...) {
   if (...) ...;
   else (...) { ... }
   if (...)
      if (...)
         if (...) ...;
         else (...) ...;
}

Then you really should consider breaking the code out to their own methods.

Spoike
+3  A: 

Check out NumberRange from commons-lang.

NumberRange range = new NumberRange(
  OPEN_FOR_RAISE_RANGE,
  OPEN_FOR_RAISE_RANGE + OPEN_FOR_CALL_RANGE
);

if(range.containsNumber(num)) {
  // do this
} else {
  // do something else
}
Tim R
+1 This is much clearer, and clarity trumps imagined performance every time. If you can demonstrate that this issue affects performance: (a) I'd be shocked; and (b) then it's time to worry about performance. Until then, clarity is most important.
Carl Manaster
A: 

If you had an enormous number of if/ifelse/ifelse/.../else statements, something like this might help:

public interface Choice {
    public boolean check(int value);
    public void action(int value);
}

public class BelowRange implements Choice {
    public static boolean check(int value) {
        return (value < 10);
    }
    public void action(int value) {
        // Do something;
    }
}

public class Range1 implements Choice {
    public boolean check(int value) {
        return (value > 10 && value < 50);
    }
    public void action(int value) {
        // Do something;
    }
}

...

And in your code:

List<Choice> choices = new ArrayList<Choice>();
choices.add(new BelowRange());
choices.add(new Range1());

...

for (Choice choice : choices) {
    if (choice.check(value)) {
        choice.action(value);
    }
}

I might implement it so the implementations of choice could be static methods, instead of having to instantiate them, but the gist of the thing is there.

If the compiler heavily optimizes this, especially with a static method, it shouldn't be slower at all, and it would make an enormously nested if/else structure a lot more maintainable from a developer's point of view.

Dean J