CODEWITHSUNDEEP
java
user1023675
user1023675

Reputation: 289

Condition handling the smart way

if (lineStyle == 5 || lineStyle == 21 || lineStyle == 82 || lineStyle == 83 || lineStyle == 3) {
    lineStyleString = "DOUBLE";
} else if (lineStyle == 6 || lineStyle == 35 || lineStyle == 39 || lineStyle == 30) {
    lineStyleString = "DOTTED" ;
} else if (lineStyle == 26 || lineStyle == 27  || lineStyle == 28  || lineStyle == 29 || lineStyle == 1) {
    lineStyleString = "SOLID";
} else if(lineStyle == -1) {
    lineStyleString = "NONE";
}

How do we handle this code the smart way in Java? Switch case, enum or key pair value mode?

Upvotes: 5

Views: 187

Answers (9)

azro
azro

Reputation: 54168

A switch case well-indented would take 30 lines (Netbeans proposed to transform it by itself so I could count)

So I would consider this way is better (9 lines) : 

if (Arrays.asList(5, 21, 82, 83, 3).contains(lineStyle)) {
     lineStyleString = "DOUBLE";
} else if (Arrays.asList(6, 35, 39, 30).contains(lineStyle)) {
     lineStyleString = "DOTTED";
} else if (Arrays.asList(26, 27, 28, 29, 1).contains(lineStyle)) {
     lineStyleString = "SOLID";
}else if (lineStyle == -1) {
     lineStyleString = "NONE";
}

Upvotes: 3

Ashish Shukla
Ashish Shukla

Reputation: 17

I think it is better to use the switch statement. 

int lcase = lineStyle;
String lineStyleString = null;
switch(lcase)
{
    case 3 :
    case 5 :
    case 21:
    case 82:
    case 83:
        lineStyleString = "DOUBLE";
        break;
    case 6 :
    case 30:
    case 35:
    case 39:
        lineStyleString = "DOTTED";
        break;
    case 1 :
    case 26:
    case 27:
    case 28:
    case 29:
        lineStyleString = "SOLID";
        break;
    case -1:
        lineStyleString = "NONE";
        break;         
}

Upvotes: -1

Eritrean
Eritrean

Reputation: 16498

Do not know if this is the smart way but it is definetly more readable and short:

lineStyleString = (lineStyle == 5 || lineStyle == 21 || lineStyle == 82 || lineStyle == 83 || lineStyle == 3)? "DOUBLE"
                    :(lineStyle == 6 || lineStyle == 35 || lineStyle == 39 || lineStyle == 30)? "DOTTED"
                    :(lineStyle == 26 || lineStyle == 27  || lineStyle == 28  || lineStyle == 29 || lineStyle == 1)? "SOLID"
                    :"NONE";

or using IntStream

   lineStyleString = (IntStream.of(5,21,82,83,3).anyMatch(x -> x == lineStyle)) ? "DOUBLE"
                    :(IntStream.of(6,35,39,30).anyMatch(x -> x == lineStyle))   ? "DOTTED"
                    :(IntStream.of(26,27,28,29,1).anyMatch(x -> x == lineStyle))? "SOLID"
                    :"NONE";

Upvotes: -1

Flown
Flown

Reputation: 11740

I prefer an enum like:

enum LineStyle {

  DOUBLE(3, 5, 21, 82, 83),
  DOTTED(6, 30, 35, 39),
  SOLID(1, 26, 27, 28, 29),
  NONE(-1);

  private final Set<Integer> types;

  private LineStyle(Integer... types) {
    this.types = Stream.of(types).collect(Collectors.toSet());
  }

  public static LineStyle of(int lineStyle) {
    return Stream.of(LineStyle.values())
      .filter(ls -> ls.types.contains(lineStyle))
      .findFirst().orElse(null);
  }
}

Then you can simply call: LineStyle ls = LineStyle.of(lineStyle);

Upvotes: 2

Elliott Frisch
Elliott Frisch

Reputation: 201497

You could prepopulate a Map<Integer, String> and save it somewhere, then use that to determine your value without a conditional check. Like,

Map<Integer, String> valueMap = new HashMap<>();
Stream.of(5, 21, 82, 83, 3).forEach(x -> valueMap.put(x, "DOUBLE"));
Stream.of(6, 35, 39, 30).forEach(x -> valueMap.put(x, "DOTTED"));
Stream.of(26, 27, 28, 29, 1).forEach(x -> valueMap.put(x, "SOLID"));
valueMap.put(-1, "NONE");

and then later

String lineStyleString = valueMap.get(lineStyle);

Upvotes: 2

Suresh Atta
Suresh Atta

Reputation: 122006

Your conditions looks more random.

Switch looks good here 

switch(lineStyle) {
    case 5:
    case 21:
    case 82:
    case 83:
    case 3: 
     lineStyleString = "DOUBLE";   
     break;
    .. // add more cases
}

Or I prefer to create utility method

public static boolean contains(int expecxted, int... vals) {
        for (int i = 0; i < vals.length; i++) {
            if (expecxted == vals[i]) {
                return true;
            }
        }
        return false;
    }

And you can use it like 

if (contains(lineStyle, 5,21,82,83,3)) {
    lineStyleString = "DOUBLE";
} else if(contains(lineStyle,6,35,39,30)){
   lineStyleString = "DOTTED";
}

Upvotes: 6

QBrute
QBrute

Reputation: 4543

You could extract the conditions into methods to make them more readable.

private boolean isDoubleStyle(int lineStyle) {
    return lineStyle == 5 || lineStyle == 21 || lineStyle == 82 || lineStyle == 83 || lineStyle == 3;
}

private boolean isDottedStyle(int lineStyle) {
    return lineStyle == 6 || lineStyle == 35 || lineStyle == 39 || lineStyle == 30;
}

private boolean isSolidStyle(int lineStyle) {
    return lineStyle == 26 || lineStyle == 27  || lineStyle == 28  || lineStyle == 29 || lineStyle == 1;
}

and then call the methods

if (isDoubleStyle(lineStyle)) {
    lineStyleString = "DOUBLE";
} else if (isDottedStyle(lineStyle)) {
    lineStyleString = "DOTTED" ;
} else if (isSolidStyle(lineStyle)) {
    lineStyleString = "SOLID";
} else {
    lineStyleString = "NONE";
}

I removed the final check for linestyle == -1 to ensure that lineStyleString always has a value, no matter what.

Upvotes: 2

Boann
Boann

Reputation: 50041

With some trivial reformatting it's fine as-is:

int s = lineStyle;

if (s == 5 || s == 21 || s == 82 || s == 83 || s == 3) {
    lineStyleString = "DOUBLE";

} else if (s == 6 || s == 35 || s == 39 || s == 30) {
    lineStyleString = "DOTTED";

} else if (s == 26 || s == 27  || s == 28  || s == 29 || s == 1) {
    lineStyleString = "SOLID";

} else if (s == -1) {
    lineStyleString = "NONE";
}

Upvotes: -2

Shinra tensei
Shinra tensei

Reputation: 1293

you could do this:

switch (lineStyle){
    case 5:
    case 21:
    case 82:
    case 83:
    case 3:
        lineStyleString = "DOUBLE";
        break;
    ...

Upvotes: 0

Related Questions