How do I (beautifully) extract this part of code to a subclass?

I have thought a lot about this but can't come to a proper solution. Also, it's hard to google things like this.

Note: The code itself is irrelevant, it's about how to do such a thing in general.

I have a class named Creature which has the following method:

```
private void move(int direction, LyxCamera camera, MapLayers mapLayers, ArrayList entities) {
if (!moving) {
if (direction != NONE) {
this.direction = direction;

int goalGridPosX = gridPosX;
int goalGridPosY = gridPosY;

if (direction == UP) {
goalGridPosY = gridPosY + 1;
} else if (direction == RIGHT) {
goalGridPosX = gridPosX + 1;
} else if (direction == DOWN) {
goalGridPosY = gridPosY - 1;
} else if (direction == LEFT) {
goalGridPosX = gridPosX - 1;
}

if (goalIsFree(goalGridPosX, goalGridPosY, mapLayers, entities)) {
stop();
moving = true;

/* FROM HERE

if (direction == UP) {
if (goalGridPosY > camera.getGridPosY() + 4) {
camera.move(direction, speed);
}
} else if (direction == RIGHT) {
if (goalGridPosX > camera.getGridPosX() + 8) {
camera.move(direction, speed);
}
} else if (direction == DOWN) {
if (goalGridPosY < camera.getGridPosY() + 4) {
camera.move(direction, speed);
}
} else if (direction == LEFT) {
if (goalGridPosX < camera.getGridPosX() + 7) {
camera.move(direction, speed);
}
}

TO HERE*/

gridPosX = goalGridPosX;
gridPosY = goalGridPosY;

step();
}

Reduce if statement nesting

You have some if statements which can be rewritten a lot better using &&/|| and early return:

if (moving || direction == NONE) {
    return; // already moving or no direction given
}

With this, we already got rid of two levels of nesting.

You can also combine the camera movement ifs:

if (direction == UP && goalGridPosY > camera.getGridPosY() + 4) {
            camera.move(direction, speed);                    
        }
        // [...etc...]

Misc

• goalGridPosX = gridPosX + 1; can be written shorter as goalGridPosX++; and goalGridPosY = gridPosY - 1; could be goalGridPosY--; (it's also less confusing. you already assigned gridPosX to goalGridPosX)

• don't hardcode magic numbers, store the values in fields instead. Right now, it's very unclear why you have 4, 8, and 7 in your code.

Move code to subclass

My first question would be: why do you want to do that? So that you can override it, because different objects behave differently with the camera?

And is there anything that speaks against the simple approach:

// replace code with:
moveCamera(direction, speed, goalGridPosX, goalGridPosY);

// add abstract method:
public abstract void moveCamera(int direction, int speed, int goalGridPosX, int goalGridPosY);

// in subclass:
@Override
public void moveCamera(int direction, int speed, int goalGridPosX, int goalGridPosY) {
    // the code
}

StackExchange Code Review Q#67089, answer score: 4