Get rid of ugly if statements

Question

I have this ugly code:

if ( v > 10 ) size = 6;
if ( v > 22 ) size = 5;
if ( v > 51 ) size = 4;
if ( v > 68 ) size = 3;
if ( v > 117 ) size = 2;
if ( v > 145 ) size = 1;
return size;

How can I get rid of the multiple if statements?

Answer 1

The obvious answer is to use Groovy:

def size = { v -> [145,117,68,51,22,10].inject(1) { s, t -> v > t ? s : s + 1 } }

One liners are always better. Returns 7 for the undefined case where v <= 10.

Answer 2

Yet another variation (less pronounced than the answer by George)

  //int v = 9;
  int[] arr = {145, 117, 68, 51, 22, 10};
  int size = 7; for(;7 - size < arr.length && v - arr[size - 2] > 0; size--) {};
  return size;

Answer 3

My commenting ability isn't turned on yet, hopefully no one will say "rightfully" based on my answer...

Pretty-ing up the ugly code could/should be defined as trying to achieve:

1. Readability (OK, stating the obvious -- redundant to the question perhaps)
2. Performance -- at best seeking optimal, at worst it's not a big drain
3. Pragmatism -- it's not far off the way most people do things, given an ordinary problem that's not in need of an elegant or unique solution, changing it later on should be a natural effort, not in need of much recollection.

IMO the answer given by org.life.java was the prettiest and extremely easy to read. I also liked the order in which the conditions were written, for reasons of reading and performance.

Looking over all the comments on this subject, at the time of my writing, it appears that only org.life.java raised the issue of performance (and maybe mfloryan, too, stating something would be "longer"). Certainly in most situations, and given this example it shouldn't bear a noticeable slowdown however you write it.

However, by nesting your conditions and optimally ordering the conditions can improve performance [worthwhile, particularly if this were looped].

All that being said, nesting and ordering conditions (that are more complex than your example) brought on by determination to achieve as fast as possible execution will often produce less readable code, and code that's harder to change. I refer again to #3, pragmatism... balancing the needs.

Answer 4

return v > 145 ? 1 
     : v > 117 ? 2 
     : v > 68 ? 3 
     : v > 51 ? 4 
     : v > 22 ? 5 
     : v > 10 ? 6 
     : "put inital size value here";

Answer 5

This is my code sample, using SortedSet. You initialise boundaries once.

SortedSet<Integer> boundaries = new SortedSet<Integer>;

boundaries.add(10);

boundaries.add(22);

boundaries.add(51);

boundaries.add(68);

boundaries.add(117);

boundaries.add(145);

Then use it subsequently this way for multiple values of v (and initialised size)

SortedSet<Integer> subset =  boundaries.tailSet(v);
if( subset.size() != boundaries.size() )
  size = subset.size() + 1;

Answer 6

It is interesting that there are plenty of beautiful answers for a simple "ugly" question. I like mfloryan's answer best, however I would push it further by removing the hard-coded array inside the method. Something like,

int getIndex(int v, int[] descArray) {
    for(int i = 0; i < descArray.length; i++)
        if(v > descArray[i]) return i + 1;
    return 0;
}

It now becomes more flexible and can process any given array in descending order and the method will find the index where the value 'v' belongs.

PS. I cannot comment yet on the answers.