r0llie
r0llie

Reputation: 69

Is there a way to refactor these if statements?

just wondering if there's a way to refactor the below code? I'm new to Java and trying to have DRY code - the below I've written but seems like a lot of conditionals to check

void printDirection() {
  if (yDirection > 0) {
    if (xDirection < 0) {
      println("Travelling South-West");
    } else {
      println("Travelling South-East");
    }
  } else if (yDirection < 0) {
    if (xDirection <0) {
      println("Travelling North-West");
    } else {
      println("Travelling North-East");
    }
  }
}

Thanks in advance for any help!

Upvotes: 3

Views: 104

Answers (3)

khelwood
khelwood

Reputation: 59210

You can evaluate the north/south and the east/west conditions individually, and glue the directions into your message.

System.out.printf("Travelling %s-%s%n", (yDirection < 0 ? "North" : "South"),
                  (xDirection < 0 ? "West" : "East"));

I assume from the code in your question that you're only concerned about those four complementary directions (not due north, due east, stationary etc.).

Upvotes: 8

Dominic Peng
Dominic Peng

Reputation: 103

Some suggestiones: 1. Due to x,y combination; there are five states; you can use enum type to define these status; 2. If you want to reduce if...else statementes in your code, please refer to Status Machine Design Pattern; but i think, under your case, the status is so simple, do not need to make it too complicated

public class Status {

    public enum Direction {
        SOUTH_WEST((x, y) -> y > 0 && x < 0, "Travelling South-West")
        , SOUTH_EAST((x, y) -> y >0 && x > 0, "Travelling South-East")
        , NORTH_EAST((x, y) -> x > 0 && y < 0, "Travelling North-East")
        , NORTH_WEST((x,y) -> x < 0 && y < 0, "Travelling North-West"), CENTER((x,y) -> x == 0 && y == 0, "");

        BiPredicate<Integer, Integer> bp;
        String desc;

        public BiPredicate<Integer, Integer> getBp() {
            return bp;
        }
        public void setBp(BiPredicate<Integer, Integer> bp) {
            this.bp = bp;

        }

        public String getDesc() {
            return desc;
        }
        public void setDesc(String desc) {
            this.desc = desc;
        }
        private Direction(BiPredicate<Integer, Integer> bp, String desc) {
            this.bp = bp;
            this.desc = desc;
        }
        public static Direction getDirection(int x, int y) {
            for (Direction direction : Direction.values()) {
                if(direction.getBp().test(x, y)) {
                    return direction;
                }
            }
            return null;
        }
    }

    public static void main(String[] args) {
        Direction d =  Direction.getDirection(3, 4);
        System.out.println(d.getDesc());
        /*      if(d == Direction.SOUTH_WEST){
                    System.out.println("do some thing");
                } else if(d == Direction.SOUTH_EAST){
                    System.out.println("do some thing");
                } else if(d == Direction.NORTH_EAST){
                    System.out.println("do some thing");
                } else if(d == Direction.NORTH_WEST){
                    System.out.println("do some thing");
                }*/
    }
}

Upvotes: 0

Arthur
Arthur

Reputation: 342

If you really want to make it DRY, it can be done using the operator ? but It's neither easy to read nor recommanded. It's used in programming contest where the goal is to go as fast as possible.

It follows the scheme : (Condition?WhatHappenIfConditionIsTrue:WhatHappenIfConditionIsFalse); You can use it in assignment :

int i = (a>0)?a:0;

in that case, if a>0 then i=a, else a=0

In your case, I would do it like that

void printDirection()
{
    System.out.println("Travelling " + (yDirection > 0?"South":"North") + "-" + (xDirection>0?"East":"West"));
}

Upvotes: 1

Related Questions