# Organizing code in procedures and functions

I'm writing code that makes the monster depend on value "followHunter", follow or run away from hunter. I have written the following code. Unfortunately it's long and non-optimized. I feel that there is a way to make it simpler by organizing it in procedures and functions but I have no idea how to do it.

In this batch of code I don't like that I have to call moveObject even if the coordinates aren't changed.

public void moveMonster(Map map, Point hunterPosition) {
Point newCoordinates;
if (followHunter) newCoordinates = followHunter(map, hunterPosition.x, hunterPosition.y);
else newCoordinates = runAwayFromHunter(map, hunterPosition.x, hunterPosition.y);
map.moveObject(x, y, newCoordinates.x, newCoordinates.y, this);
}


Function checkCollision returns true when monster can move on specified coordinates. When monster runs away from hunter, direction is random. Then when monster encounters an obstruction it's change direction and tries again. When it encounters an obstruction again then it doesn't move.

private Point runAwayFromHunter(Map map, int hunterX, int hunterY) {
int x = this.x;
int y = this.y;
boolean secondAttempt = false;
boolean direction = random.nextBoolean();

if (direction) {
if (x < hunterX) {
if (map.checkCollision(x-1, y)) --x;
else {
direction = !direction;
secondAttempt = true;
}
} else {
if (map.checkCollision(x+1, y)) ++x;
else {
direction = !direction;
secondAttempt = true;
}
}
} else {
if (y < hunterY) {
if (map.checkCollision(x, y-1)) --y;
else {
direction = !direction;
secondAttempt = true;
}
} else {
if (map.checkCollision(x, y+1)) ++y;
else {
direction = !direction;
secondAttempt = true;
}
}
}
if (secondAttempt) {
if (direction) {
if (x < hunterX) {
if (map.checkCollision(x-1, y)) --x;
} else {
if (map.checkCollision(x+1, y)) ++x;
}
} else {
if (y < hunterY) {
if (map.checkCollision(x, y-1)) --y;
} else {
if (map.checkCollision(x, y+1)) ++y;
}
}
}
return new Point(x,y);
}


When the monster follows the hunter; if its in distance of one grid of the hunter then the monster doesn't move. When it's on the same horizontal position it moves vertically, when it's on the same vertical position it moves horizontally. Otherwise the selected direction is random. When monster encounters an obstruct it changes direction and tries again. When the monster encounters an obstruction again then the monster doesn't move.

private Point followHunter(Map map, int hunterX, int hunterY) {
boolean secondAttempt = false;
boolean direction;
int x = this.x;
int y = this.y;

if (Math.abs(hunterX - x) <= 1 && Math.abs(hunterY - y) <= 1) return new Point(x,y);

if (x == hunterX) direction = false;
else if (y == hunterY) direction = true;
else direction = random.nextBoolean();

if (direction) {
if (x < hunterX) {
if (map.checkCollision(x+1, y)) ++x;
else {
direction = !direction;
secondAttempt = true;
}
} else {
if (map.checkCollision(x-1, y)) --x;
else {
direction = !direction;
secondAttempt = true;
}
}
} else {
if (y < hunterY) {
if (map.checkCollision(x, y+1)) ++y;
else {
direction = !direction;
secondAttempt = true;
}
} else {
if (map.checkCollision(x, y-1)) --y;
else {
direction = !direction;
secondAttempt = true;
}
}
}
if (secondAttempt) {
if (direction) {
if (x < hunterX) {
if (map.checkCollision(x+1, y)) ++x;
} else {
if (map.checkCollision(x-1, y)) --x;
}
} else {
if (y < hunterY) {
if (map.checkCollision(x, y+1)) ++y;
} else {
if (map.checkCollision(x, y-1)) --y;
}
}
}
return new Point(x,y);
}


As you can see there is a lot of repeated code. I would be grateful for any tips, suggestions and samples of optimized code.

I've rewritten your code below. The biggest change I made was to combine all the move logic into a separate function move. Both of your functions did nearly the same thing so I pulled out the logic and moved it into a separate function. The next biggest change I made was to remove the "second attempt" logic and just retry in the opposite direction.

private Point move(Map map, boolean direction, boolean towardsHunter) {
int x = this.x;
int y = this.y;
int dx = 0;
int dy = 0;

if (direction)
dx = (x < hunterX) ? 1 : -1;
else
dy = (y < hunterY) ? 1 : -1;

if (!towardsHunter) {
dx = -dx;
dy = -dy;
}

// First attempt.
if (map.checkCollision(x+dx, y+dy))
return new Point(x+dx, y+dy);

// Second attempt in the opposite direction.
if (map.checkCollision(x-dx, y-dy))
return new Point(x-dx, y-dy);

// Couldn't move, so stay put.
return new Point(x, y);
}

private Point runAwayFromHunter(Map map, int hunterX, int hunterY) {
return move(map, random.nextBoolean(), false);
}

private Point followHunter(Map map, int hunterX, int hunterY) {
boolean direction;
int x = this.x;
int y = this.y;

if (Math.abs(hunterX - x) <= 1 && Math.abs(hunterY - y) <= 1) return new Point(x,y);

if (x == hunterX) direction = false;
else if (y == hunterY) direction = true;
else direction = random.nextBoolean();

return move(map, direction, true);
}


This could be simplified:

    if (x < hunterX) {
if (map.checkCollision(x-1, y)) --x;
else {
direction = !direction;
secondAttempt = true;
}
} else {
if (map.checkCollision(x+1, y)) ++x;
else {
direction = !direction;
secondAttempt = true;
}
}


Basically, depending on the condition x < hunterX, you either check-and-add 1 or check-and-subtract 1 to x. This code is equivalent, with less duplication:

    int dx = x < hunterX ? -1 : 1;
if (map.checkCollision(x + dx, y)) x += dx;
else {
direction = !direction;
secondAttempt = true;
}


You could apply the same for the other checks on y < hunterY.

Next, you could apply the same logic inside if (secondAttempt) { ... }.

Next, you could copy and paste the now simplified code of if (secondAttempt) { ... } into the first set of conditions where you set secondAttempt = true, and further simplify that too. As a result, the method body becomes this:

    int dx = x < hunterX ? -1 : 1;
int dy = y < hunterY ? -1 : 1;

if (direction) {
if (map.checkCollision(x + dx, y)) x += dx;
else if (map.checkCollision(x, y + dy)) y += dy;
} else {
if (map.checkCollision(x, y + dy)) y += dy;
else if (map.checkCollision(x + dx, y)) x += dx;
}

return new Point(x, y);


This is simpler and easier to understand.

You could apply the same technique in followHunter too.

I recommend to use a different name for local variables than member variables, to avoid possible confusion and errors. For example, instead of int x = this.x, how about int newX = this.x.