Brett’s Refactoring Exercise Solution Take 1
Recently Brett Schuchert from Object Mentor has started posting code snippets on his blog and inviting people to refactor it. Similar to the Daily Refactoring Teaser that I’m conducting at Directi. (I’m planning to make it public soon).
Following is my refactored solution (take 1) to the poorly written code.
Wrote some Acceptance Test to understand how the RpnCalculator works:
Then I updated the perform method to
@Deprecated public void perform(final String operatorName) { perform(operators.get(operatorName)); } public void perform(final Operator operator) { operator.eval(stack); currentMode = Mode.inserting; } |
Notice I’ve deprecated the old method which takes String. I want to kill primitive obsession at its root.
Had to temporarily add the following map (this should go away once our deprecated method is knocked off).
private static Map operators = new HashMap() { { put("+", Operator.ADD); put("-", Operator.SUBTRACT); put("!", Operator.FACTORIAL); } @Override public Operator get(final Object key) { if (!super.containsKey(key)) { throw new MathOperatorNotFoundException(); } return super.get(key); } }; |
Defined various Operators
private static abstract class Operator { private static final Operator ADD = new BinaryOperator() { @Override protected int eval(final int op1, final int op2) { return op1 + op2; } }; private static final Operator SUBTRACT = new BinaryOperator() { @Override protected int eval(final int op1, final int op2) { return op2 - op1; } }; private static final Operator FACTORIAL = new UnaryOperator() { @Override protected int eval(final int op1) { int result = 1; int currentOperandValue = op1; while (currentOperandValue > 1) { result *= currentOperandValue; --currentOperandValue; } return result; } }; public abstract void eval(final OperandStack stack); } |
Declared two types of Operators (BinaryOperator and UnaryOperator) to avoid duplication and to make it easy for adding new operators.
public static abstract class BinaryOperator extends Operator { @Override public void eval(final OperandStack s) { s.push(eval(s.pop(), s.pop())); } protected abstract int eval(int op1, int op2); } |
and
public static abstract class UnaryOperator extends Operator { @Override public void eval(final OperandStack s) { s.push(eval(s.pop())); } protected abstract int eval(int op1); } |
