intruder
intruder

Reputation: 417

If else redundant code to a simpler form

How to represent the following if else condition in a better way?

        if ((!StringUtils.isEmpty(A) && !StringUtils.isEmpty(B)) {
            System.out.println("Case 1");
        } else if ((!StringUtils.isEmpty(A) && StringUtils.isEmpty(B)) {
            System.out.println("Case 2");
        } else if ((StringUtils.isEmpty(A) && !StringUtils.isEmpty(B)) {
            System.out.println("Case 3");
        } else if ((StringUtils.isEmpty(A) && StringUtils.isEmpty(B)) {
            System.out.println("Case 4");
        } else {
            System.out.println("End");
        }

Upvotes: 4

Views: 399

Answers (3)

sidgate
sidgate

Reputation: 15234

I would suggest to have smaller functions for each condition for better readability.

Also, you don't need fifth condition, and reordering the conditions make it more readable

    import static <pkg>.StringUtils.isEmpty;

    if (bothEmpty(A,B)) {
        System.out.println("Case 4");
    } else if (isEmpty(A)) {
       System.out.println("Case 2");
    } else if (isEmpty(B)) {
        System.out.println("Case 3");
    } else  {
       System.out.println("Case 1");
    }

  boolean bothEmpty(String A, String B){
     return isEmpty(A) && isEmpty(B);
  }

I usually return the value if a condition satisfies. This avoids some duplication and nested if-else cases.

 public originalAction(String A, String B){
     System.out.println(value(A,B));
 }
 String value(String A, String B){
    if (bothEmpty(A,B))  return "Case 4";
    if (isEmpty(A)) return "Case 2";
    if (isEmpty(B)) return "Case 3";
    return "Case 1";
}
boolean bothEmpty(String A, String B){
   return isEmpty(A) && isEmpty(B);
}

Upvotes: 1

Dawood ibn Kareem
Dawood ibn Kareem

Reputation: 79818

This is the clearest way I can think of, to write your example.

Note that I've used the standard Java convention of lower case letters for identifiers. If you really have named your variables A and B, for whatever reason, you might consider using more standard names.

import static org.apache.commons.lang3.StringUtils.isEmpty;

...

if (isEmpty(a)) {
    if (isEmpty(b)) {
        System.out.println("Case 4");
    } else {
        System.out.println("Case 3");
    }
} else {
    if (isEmpty(b)) {
        System.out.println("Case 2");
    } else {
        System.out.println("Case 1");
    }
}

Upvotes: 3

Andreas
Andreas

Reputation: 159086

Assuming Case 1 and Case 4 are not supposed to have the same condition, which eliminates the possibility of End, you can do this:

if (StringUtils.isEmpty(A)) {
    if (StringUtils.isEmpty(B)) {
        System.out.println("Case 4");
    } else {
        System.out.println("Case 3");
    }
} else {
    if (StringUtils.isEmpty(B)) {
        System.out.println("Case 2");
    } else {
        System.out.println("Case 1");
    }
}

Or you can use a switch statement, with a little bit-math:

switch ((StringUtils.isEmpty(A) ? 2 : 0) | (StringUtils.isEmpty(B) ? 1 : 0)) {
    case 0: // !empty(A) & !empty(B)
        System.out.println("Case 1");
        break;
    case 1: // !empty(A) & empty(B)
        System.out.println("Case 2");
        break;
    case 2: // empty(A) & !empty(B)
        System.out.println("Case 3");
        break;
    default: // empty(A) & empty(B)
        System.out.println("Case 4");
}

Upvotes: 4

Related Questions