Creating restrictions for a sprite's possible position on the screen

Question

The great feedback I got on my first post on Code Review made me wary of code repetition. This code does what it is supposed to, which is to create restrictions for a sprite's possible position on the screen.

Is this a neat looking solution, or can it be considered code repetition?

public void move(int x, int y) {
    int maxX = screenWidth - image.getWidth()/2;
    int minX = image.getWidth() / 2;
    int maxY = screenHeight - image.getHeight()/2;
    int minY = image.getHeight() / 2;

    if (x > maxX)
        this.x = screenWidth - image.getWidth();
    else if (x < minX)
        this.x = 0;
    else
        this.x = x - image.getWidth() / 2;

    if (y > maxY)
        this.y = screenHeight - image.getHeight();
    else if (y < minY)
        this.y = 0;
    else
        this.y = y - image.getHeight() / 2;
}

Answer 1

It is possible to refactor your code starting from the definition of your variables:

int maxX = screenWidth - image.getWidth()/2;
int minX = image.getWidth() / 2;
int maxY = screenHeight - image.getHeight()/2;
int minY = image.getHeight() / 2;

You are repeating function calls image.getWidth(), image.getHeight() and divisions by 2 in more than one definition, while you could call store functions calls results and division results in other variables like below:

int width = image.getWidth();
int height = image.getHeight();
int minX = width / 2;
int minY = height / 2;
int maxX = screenWidth - minX;
int maxY = screenHeight - minY;

The other code you can simplify is the following:

if (x > maxX)
        this.x = screenWidth - image.getWidth();
    else if (x < minX)
        this.x = 0;
    else
        this.x = x - image.getWidth() / 2;
    // same behaviour of the above code
    if (y > maxY)
        this.y = screenHeight - image.getHeight();
    else if (y < minY)
        this.y = 0;
    else
        this.y = y - image.getHeight() / 2;

It is the same code applied one time to width and other time to height : you could define a helper function to be applied two times like below:

private int getNewCoordinate(int c, int l, int max, int min, int maxl) {
        if (c > max) { return maxl - l; }
        if (c < min) { return 0; }
        return c - l / 2;
}

Now if I haven't made confusion with some variable your original code can be written like this:

public void move(int x, int y) {
    int width = image.getWidth();
    int height = image.getHeight();
    int minX = width / 2;
    int minY = height / 2;
    int maxX = screenWidth - minX;
    int maxY = screenHeight - minY;

    this.x = getNewCoordinate(x, width , maxX, minX, screenWidth);
    this.y = getNewCoordinate(y, height, maxY, minY, screenHeight);   
}

private int getNewCoordinate(int c, int l, int max, int min, int maxl) {
    if (c > max) { return maxl - l; }
    if (c < min) { return 0; }
    return c - l / 2;
}