# Common factor in some cases [closed]

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.

• Could you provide some more context about this code? What is a and e? What does and though? What does expr(j).reduce(C1) do? You don't need to provide the code for them, just explain what it is doing and what types you are using. Dec 14, 2013 at 16:40
• Are a, e booleans or some more exotic structure? Dec 14, 2013 at 17:20
• I agree with @SimonAndréForsberg, and I suspect you may have an XY problem. It looks like you are trying to do arithmetic based on a string representation of some numbers. Maybe BigInteger is appropriate? I encourage you to post the entire function, probably as a separate question referencing this one. Dec 14, 2013 at 17:30
• @Val if (c == '-') handleThatcase(); else duplicateOfYourPreviousQuestion(); Dec 14, 2013 at 18:11
• @SimonAndréForsberg You are right.
– Val
Dec 14, 2013 at 18:42

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

• Your idea to use extra if-else right in the fallthrough case is better than mine to pull if-else into the and. But, it does not resolve my concern that if-else for invert must be resolved by cases and using extra if-else is bad (cyclomatic complexity, pipeline stalls and discards). I know what is the ternary operator. It is the same as if-else and I do not think that it is better than if-else. Thanks.
– Val
Dec 14, 2013 at 16:50
• @Val Your question was quite unclear so it was hard to tell what level of experience you had, and what you are actually asking. Dec 14, 2013 at 17:13
• Not clear? Do you see that not in the common pattern? It is not ok. Merging the cases with additional if is also not ok.
– Val
Dec 14, 2013 at 17:53

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'

• reduce is just a placeholder for some another function. Sorry for a confusing name.
– Val
Dec 14, 2013 at 18:26
• LSB of c-'0' is 0 or 1 for cases 0 and 1. But (c-'0' >> 1 != 0) passes if c is 2 or larger. How do your single out cases 0 and 1 this way?
– Val
Dec 14, 2013 at 18:40
• @Val - it works... do the math: See my edit: Dec 14, 2013 at 18:43
• @Val perhaps it should be >> 1 == 0` Dec 14, 2013 at 18:44
• Ahh... yes.... copy-paste problem/ Dec 14, 2013 at 18:45