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;
}

• Rob Audenaerde: You certainly can reuse minX and minY more

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;
}