Common factor in some cases [closed]

Question

I have this code:

switch (c) {
    case '-': break;
    case '0': e = expr(j).reduce(C1); e=not(e); a = and(a, e); break;
    case '1': e = expr(j).reduce(C1); a = and(a, e); break;
    default: throw new Exception("Unexpected cube value, " + c);
}

The cases 0 and 1 are only different by not. I do not like to encode the necessity to invert into the arguments of the and function,

switch (c) {
    case '-': break;
    case '0': a = and(true, a, e); break;
    case '1': a = and(false, a, e); break;
    default: throw new Exception("Unexpected cube value, " + c);
}

because this will need a conditional if (invert) e = inv(e); in the function, whereas we already established the need to invert when resolved the switch.

I mean that in case1 you know that you do not need any inv. In case0, you know that you need one. If you now merge the cases, you will need one more if to resolve between case1 (inv is not needed) and case0 (inv is needed) once again.

Answer 1

There are two significantly different ways I can think of to rewrite your code.

The first is to put case '0' and case '1' together by letting one case 'fall through', and then using the ternary operator with in the code to see if we should invert or not.

switch (c) {
    case '-': break;
    case '0': // fall through to execute the same code as below
    case '1': 
         e = expr(j).reduce(C1);
         // Using the ternary operator to determine whether or not e should go through not first.
         a = and(a, c == '0' ? not(e) : e);
         break;
    default: throw new Exception("Unexpected cube value, " + c);
}

The other approach is to skip the switch and use if-else

if (c == '0' || c == '1') {
     e = expr(j).reduce(C1);
     a = and(a, c == '0' ? not(e) : e); // Again, using the ternary operator.
}
else if (c != '-') 
    throw new Exception("Unexpected cube value, " + c);

There might be even better ways to rewrite this code, if I would know more context about it.

I agree that you should not move the logic of inverting e to the and method.

Explanation of the ternary operator:

a = and(a, c == '0' ? not(e) : e);

This is the same thing as:

if (c == '0')
     a = and(a, not(e));
else 
     a = and(a, e);

A more detailed explanation can be found on wikipedia

Answer 2

Following on from Simon's answer (which I believe will be more than great for 99.99% of users).....

... actually, I have looked though the code, and I don't understand enough of it to make sense of the right answer:

• why do you do the map-reduce at all if a = and(a,e) will be 'false' if a starts off 'false`?
• the map-reduce (two method calls) are far, far more expensive than the switch and conditionals outside the methods... this loop does not need to be optimized any more.
• The 'boolean-like' variables a, e are not well described at all.

Without knowing more about the code and it's context, I can think of only one thing that may help, and this is an extension of Simon's answer:

if (((c - '0')) >> 1 == 0) { // reduce cyclomatic complexity by 1.
    // '0' is 0x30, (c - '0') will end up with 0 for '0' and 1 for '1' - anything else is wrong
    e = expr(j).reduce(C1);
    a = and(a, c == '0' ? not(e) : e); // Again, using the ternary operator.
} else if (c != '-') 
    throw new Exception("Unexpected cube value, " + c);

Any other suggestions would just be wild guesses.

EDIT

public static void char01() {
    for (char c = Character.MIN_VALUE; c <= Character.MAX_VALUE; c++) {
        if ((c - '0') >> 1 == 0) {
            System.out.println("This matches '" + c + "'");
        }
    }
}

produces:

This matches '0'
This matches '1'