public class LogicMatrix {
public static void callTheRightMethod(boolean a,
boolean b,
boolean c,
boolean d,
Interface callback) {
// TODO: Refactor this to be simple and obvious:
if (!a && !b && !c && !d)
callback.x();
else if (a && !b && !c && !d)
callback.z();
else if (!a && b && !c && !d)
callback.w();
else if (a && b && !c && !d)
callback.w();
else if (!a && !b && c && !d)
callback.x();
else if (a && !b && c && !d)
callback.x();
else if (!a && b && c && !d)
callback.z();
else if (a && b && c && !d)
callback.w();
else if (!a && !b && !c && d)
callback.y();
else if (a && !b && !c && d)
callback.x();
else if (!a && b && !c && d)
callback.x();
else if (a && b && !c && d)
callback.z();
else if (!a && !b && c && d)
callback.y();
else if (a && !b && c && d)
callback.y();
else if (!a && b && c && d)
callback.x();
else if (a && b && c && d)
callback.x();
}
}
This is based on a "decision matrix" that Jeff included for a bit of interesting code generation and test generation in his JUnit test for the class.
This matrix was really a string:
String matrix = "" //
+ "XZWW" //
+ "XXZW" //
+ "YXXZ" //
+ "YYXX";
His post asked us to make this code more resemble the decision matrix "without lots of reflection or other icky stuff". My tactic for doing this is to use the "decision matrix" itself as data to drive the code.
I found this example interesting because the problem is mainly in noticing a little math hiding in the repeated patterns. It's not too hard to see, but a refactor can certainly make it more visible in the code. It being Sunday, I have some free time, so I downloaded it and fired up Eclipse.
Jeff's tests all passed of course ...
First off, notice that if we interpret false as 0 and true as 1, the conditions of the above long method can be viewed as the binary numbers 0 through 15 (conveniently in the right order), and the method calls are running through the corresponding letters in the matrix. Not of course surprising, as it was code-generated from the matrix.
So let's make a little function that turns those booleans into indexes:
private static int getIndex(boolean a,
boolean b,
boolean c,
boolean d) {
int result = 0;
result |= (a ? 1:0);
result |= (b ? 2:0);
result |= (c ? 4:0);
result |= (d ? 8:0);
return result;
}
And change that monster method to:
public static void callTheRightMethod(boolean a,
boolean b,
boolean c,
boolean d,
Interface callback) {
int index = getIndex(a,b,c,d);
if (index==0)
callback.x();
else if (index==1)
callback.z();
else if (index==2)
callback.w();
else if (index==3)
callback.w();
else if (index==4)
callback.x();
else if (index==5)
callback.x();
else if (index==6)
callback.z();
else if (index==7)
callback.w();
else if (index==8)
callback.y();
else if (index==9)
callback.x();
else if (index==10)
callback.x();
else if (index==11)
callback.z();
else if (index==12)
callback.y();
else if (index==13)
callback.y();
else if (index==14)
callback.x();
else if (index==15)
callback.x();
}
The tests still pass.
That method's still long, but at least it's now obvious there's a sequence of indices going on.
So we put that matrix into the code as a constant, use our indices, and the long method becomes:
public static void callTheRightMethod(boolean a,
boolean b,
boolean c,
boolean d,
Interface callback) {
int index = getIndex(a,b,c,d);
char entry = matrix.charAt(index);
if (entry == 'W')
callback.w();
else if (entry == 'X')
callback.x();
else if (entry == 'Y')
callback.y();
else if (entry == 'Z')
callback.z();
}
The code has suddenly shrunk. I could of course shrink it a bit more with reflection, but that's "icky", so I'll just leave it at that.
But I think there's a bit of duplication hiding in the getIndex function...
Lets try putting those parameters into an array and overloading (even though overloads are a little icky) :
private static int getIndex(boolean a,
boolean b,
boolean c,
boolean d) {
return getIndex(new boolean[] {a,b,c,d});
}
private static int getIndex(boolean[] args) {
int result = 0;
result |= (args[0] ? 1:0);
result |= (args[1] ? 2:0);
result |= (args[2] ? 4:0);
result |= (args[3] ? 8:0);
return result;
}
And now the duplication is obvious - at least I think it's obvious ...
private static int getIndex(boolean[] args) {
int result = 0;
for (int i=0; i<4; i++)
result |= bitForBooleanAtIndex(args, i);
return result;
}
private static int bitForBooleanAtIndex(boolean[] args, int i) {
return (args[i] ? 1<<i:0);
}
That's about it. It's definitely clearer. Whether it meets the purpose of Jeff's challenge I'll leave to Jeff. I inlined some stuff to but I'll skip reposting everything.
There's some duplication between test and code now... At least they both have the "decision matrix".
For this to really be useful, it should probably be a class with instance methods and the matrix as a constructor parameter (which could eliminate that bit of duplication with the test code). It should then maybe allow for a differently sized decision matrix. I think it would need reflection to be generally useful, with the callback class and/or method list maybe also injected by constructor. But I'm not going there right now.
Anyway, now this very young blog has some actual code in it.
No comments:
Post a Comment