Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

I agree with @rolfl that you need to extract methods and use a decision table decision table.

I agree with @rolfl that you need to extract methods and use a decision table.

I agree with @rolfl that you need to extract methods and use a decision table.

Source Link
palacsint
  • 30.4k
  • 9
  • 82
  • 157

I agree with @rolfl that you need to extract methods and use a decision table.

From Code Complete 2nd Edition, Chapter 19: General Control Issues, page 431:

Use decision tables to replace complicated conditions

Sometimes you have a complicated test involving several variables. It can be helpful to use a decision table to perform the test rather than using ifs or cases. A decision-table lookup is easier to code initially, having only a couple of lines of code and no tricky control structures. This minimization of complexity minimizes the opportunity for mistakes. If your data changes, you can change a decision table without changing the code; you only need to update the contents of the data structure.

I would be more verbose here for the sake of readability. First, an enum for the possible differences:

public enum Difference {
 ZERO,
 BELOW_ZERO,
 ABOVE_ZERO
}

Then a class which contains the available rules:

public class MoveLogic {
 private final Map<Rule, Move> rules = new LinkedHashMap<>();
 public MoveLogic() {
 addRule(Difference.ABOVE_ZERO, Difference.ABOVE_ZERO, 30, 30); // 8
 addRule(Difference.ABOVE_ZERO, Difference.BELOW_ZERO, 30, -30); // 3
 addRule(Difference.ABOVE_ZERO, Difference.ZERO, 30, 0); // 5
 addRule(Difference.BELOW_ZERO, Difference.ABOVE_ZERO, -30, 30); // 6
 addRule(Difference.BELOW_ZERO, Difference.BELOW_ZERO, -30, -30); // 1
 addRule(Difference.BELOW_ZERO, Difference.ZERO, -30, 0); // 4
 addRule(Difference.ZERO, Difference.BELOW_ZERO, 0, -30); // 7
 addRule(Difference.ZERO, Difference.ABOVE_ZERO, 0, 30); // 2
 }
 private void addRule(Difference xDifference, Difference yDifference, int x, int y) {
 rules.put(new Rule(xDifference, yDifference), new Move(x, y));
 }
 public Move getMove(Difference xDifference, Difference yDifference) {
 return rules.get(new Rule(xDifference, yDifference));
 }
}

Usage:

final Difference xDifference = calculateDifference(plant.getXpos(), xpos);
final Difference yDifference = calculateDifference(plant.getXpos(), ypos);
final Move move = moveLogic.getMove(xDifference, yDifference);
xpos += move.getX();
ypos += move.getY();

Finally, the helper method to create Difference references:

private Difference calculateDifference(final int plantPosition, final int position) {
 final int difference = plantPosition - position;
 if (difference > 0) {
 return Difference.ABOVE_ZERO;
 }
 if (difference < 0) {
 return Difference.BELOW_ZERO;
 }
 return Difference.ZERO;
}

as well as Rule:

public class Rule {
 private Difference xDifference;
 private Difference yDifference;
 public Rule(Difference xDifference, Difference yDifference) {
 this.xDifference = xDifference;
 this.yDifference = yDifference;
 }
 // generate hashCode and equals, the Map needs it!
}

and Move:

public class Move {
 private int x;
 private int y;
 public Move(int x, int y) {
 this.x = x;
 this.y = y;
 }
 public int getX() {
 return x;
 }
 public int getY() {
 return y;
 }
}

It might seem too verbose but it avoids repeated conditions as well as magic computations with indexes and arrays.

lang-java

AltStyle によって変換されたページ (->オリジナル) /