Jacques Celliers
Jacques Celliers

Reputation: 51

Java If and else simplification

I have this very long if and else statement, any idea how i would be able to shorten this?

Or is this going to be the only way for me to handle this?

if (HR < 41) {
    HR_Score = 2;
} else if (HR < 51) {
    HR_Score = 1;
} else if (HR < 101) {
    HR_Score = 0;
} else if (HR < 111) {
    HR_Score = 1;
} else if (HR < 129) {
    HR_Score = 2;
} else {
    HR_Score = 3;
}

Upvotes: 0

Views: 220

Answers (5)

acegs
acegs

Reputation: 2809

you can also look at my version of solutions.

//solution1:
     if (HR < 41)  HR_Score = 2;
else if (HR < 51)  HR_Score = 1;
else if (HR < 101) HR_Score = 0;
else if (HR < 111) HR_Score = 1;
else if (HR < 129) HR_Score = 2;
else               HR_Score = 3;


//solution2:
if (HR < 41)  HR_Score = 2; else 
if (HR < 51)  HR_Score = 1; else 
if (HR < 101) HR_Score = 0; else 
if (HR < 111) HR_Score = 1; else 
if (HR < 129) HR_Score = 2; else 
              HR_Score = 3;


//no branch solution.
HR_Score = 3 - (HR < 129)
             - (HR < 111)
             - (HR < 101)
             + (HR < 51)
             + (HR < 41);
             

Upvotes: 0

Thatalent
Thatalent

Reputation: 424

You can use the ? : operator to make the code shorter:

int HR_Score = HR < 41   ? 2 : 
              (HR < 51)  ? 1 : 
              (HR < 101) ? 0 : 
              (HR < 111) ? 1 : 
              (HR < 129) ? 2 :
                           3;

The conditional operator (aka "ternary" because it takes three operands) is a convenient way of chaining if/else statements that you don't care to nest.

Upvotes: 5

Tschallacka
Tschallacka

Reputation: 28722

EDIT I recommend using status answer over this. Leaving this here because it poses an alternative way to get tot he same result.

You can use a Sorted Map with Integers defined by key.

Since you have an exit early strategy, a value can only come in place when the previous tests have failed, we can return early in a loop.

A map exits out of pairs, a key and a value. You can iterate these key value sets and then use that entry set as a comparing function.

I used TreeMap below as implementation, because then we can be relatively sure that the sort order is the order of insertion.

See it live: https://ideone.com/3J3Fqs

import java.util.*;
import java.lang.*;
import java.io.*;


class Ideone
{
    public static void main (String[] args) throws java.lang.Exception
    {
        Ideone one = new Ideone();
        System.out.println("41:2="+one.getScore(41));
        System.out.println("50:1="+one.getScore(50));
        System.out.println("101:0="+one.getScore(101));
        System.out.println("110:1="+one.getScore(110));
        System.out.println("128:2="+one.getScore(128));
        System.out.println("200:3="+one.getScore(200));
    }
        private SortedMap<Integer, Integer> pairList = null;

       public SortedMap<Integer, Integer> getPairList() {
           if(pairList == null) {
               pairList = new TreeMap<Integer, Integer>();
               pairList.put(42,2);
               pairList.put(51,1);
               pairList.put(101,0);
               pairList.put(111,1);
               pairList.put(129,2);
           }
           return pairList;
       }

    public int getScore(int compare) {
       for(Map.Entry<Integer, Integer> entry : this.getPairList().entrySet()) {
           Integer key = entry.getKey();
           if(compare < key) {
               return entry.getValue();
           }

       }
       return 3;

    }
}

As an unrelated comment: I suggest you stay away from "magic numbers"/"hardcoded numbers". Try to see if you can put them in a settings file or a database and load them from there. This will make your application more future proof and easier to change values without having to recompile and redeploy.

Upvotes: 0

T.J. Crowder
T.J. Crowder

Reputation: 1074168

If it's really just those six possibilities, that if/else is just fine, and I wouldn't expect any of the alternatives to be clearer.

I'd stick with the if/else.

One example of an alternative that's not better: :-)

If HR has a relatively narrow range, you could do a lookup table instead.

private static final int[] scores = {2, 2, 2, 2, 2, /*...*/ 1, /*...*/ 0, /*...*/};

then when you need a score:

if (HR >= 0 && HR < scores.length) {
    HR_Score = scores[HR];
}

..but that's more complex than what you have, and harder to read.

Alternately, a map giving the range and value:

private static int[][] scoreLookup = {
    {41, 2},
    {51, 1},
    {101, 0},
    {111, 1},
    {129, 2},
    {Integer.MAX_VALUE, 3}
};

and then

for (int[] entry : scoreLookup) {
    if (HR < entry[0]) {
        HR_Score = entry[1];
        break;
    }
}

...but I still prefer your if/else.

Upvotes: 0

statut
statut

Reputation: 909

You can solve your problem with NavigableMap. For example:

// In the class
private static final NavigableMap<Integer, Integer> map = new TreeMap<>();
map.put(41, 2);
map.put(51, 1);
map.put(101, 0);
map.put(111, 1);
map.put(129, 2);
map.put(Integer.MAX_VALUE, 3);

// When you need a score
HR_Score = map.ceilingEntry(HR).getValue();

Upvotes: 10

Related Questions