Removing duplicate code from basic collision detection implementation The code is part of a very basic 2D platformer based on code from the book Killer Game Programming in Java.

The methods are part of a TileMapManager class which is handling basic collision detection.


The first method returns the greatest distance that an entity can travel in the y axis (up to deltaY) without colliding with a tile:

public int getMaxDeltaY(Point initialPoint, int deltaY){

    //validate params
    if(deltaY&gt;=tileMap.getTileHeight()) throw new IllegalArgumentException("Cannot validate if deltaY is larger than brickHeight");
    if(deltaY==0) return deltaY;

    int x = initialPoint.x;
    int testY = initialPoint.y + deltaY;

    if(isInsideTile(x, testY)){

        //get which row we are colliding with
        int mapRow = testY/tileMap.getTileHeight();
        //the distance between our position and
        //the top of the tile we are colliding with
        int topOffset = testY - (mapRow * tileMap.getTileHeight());
        //get the amount we should move to place
        //ourselves just next to the tile (top or bottom)
        int maxDeltaY = 0;

        //moving up case
        if(deltaY &lt;0){
            maxDeltaY = deltaY + (tileMap.getTileHeight()-topOffset);
        }
        //moving down case
        else if(deltaY &gt;0){
            maxDeltaY = deltaY - topOffset;
        }
        return maxDeltaY;
    }
    //we won't collide so it's okay to move deltaY
    return deltaY;
}

The second method does the same thing in the x axis:

public int getMaxDeltaX(Point initialPoint, int deltaX){
    //validate params
    if(deltaX&gt;=tileMap.getTileWidth()) throw new IllegalArgumentException("Cannot validate if moveStep is larger than brickWidth");
    if(deltaX==0) return deltaX;

    int testX = (initialPoint.x + deltaX);
    int y = initialPoint.y;

    //if we collide
    if(isInsideTile(testX, y)){

        //get which column we are colliding with
        int mapCol = testX/tileMap.getTileWidth();
        //the distance between our position and
        //the left side of the tile we are colliding with
        int leftOffset = testX - (mapCol * tileMap.getTileWidth());
        //get the amount we should move to place
        //ourselves just next to the tile (left or right)
        int maxDeltaX = 0;

        //moving left
        if(deltaX &lt; 0){
            maxDeltaX = deltaX + (tileMap.getTileWidth() - leftOffset);
        }
        //moving right
        else if(deltaX &gt; 0){
            maxDeltaX = deltaX - leftOffset;
        }

        return maxDeltaX;
    }
    //we won't collide so it's okay to move deltaX
    return deltaX;
}



The algorithm (from the book) is essentially the same so I wanted to remove the duplication.

The code differs in only a few places:


use getTileWidth() or getTileHeight()
use x or y to create test point
use x or y to create offset
use mapRow or mapCol


So a basic way could be:


pass tile width/height as argument tileDimension
create test point before calling method (turn initialPoint into testPoint)
pass test x/y as argument testCoordinate
pass row/col as argument?  Can't think of good name (mapIndex?)


But now I have loads of confusing arguments and duplicate information (testCoordinate is contained in testPoint). I read that this is also a big no no, and even a beginner like me can see that this would be ugly.

Another way might be to pass the basic information and a flag for the axis, x or y. But this would still involve a bunch of arguments and then confusing switching in the body.

So I'm hoping there's a simple pattern that is used in these situations and will magically get rid of all duplication and leave a short method that's easy to read and use...

What is the best way to refactor these methods?
It seems this is a job for a Parameter class.

public abstract class DeltaParameters{
  protected ? tileMap;
  protected int x, y;

  DeltaParameters(Point point, ? tileMap){
    this.x = point.x;
    this.y = point.y;
    this.tileMap = tileMap;
  }

  public int getX(){
    return x;
  }

  public int getY(){
    return y;
  }
  public abstract int getTileSideSize();
  public abstract int getMovingCoordinate();
  public abstract void offsetMovingCoordinate(int delta);
}

public class DeltaXParameters extends DeltaParameters{
  DeltaXParameters(Point point, ? tileMap){
    super(point, tileMap);
  }

  public int getTileSideSize(){
    return tileMap.getTileWidth();
  }
  public int getMovingCoordinate(){
    return x;
  }
  public void offsetMovingCoordinate(int delta){
    x += delta;
  }
}


I leave to you the job of guessing what would be DeltaYParameters :p 

public int getMaxDelta(DeltaParameters dparams, int delta){
    if(delta&gt;=dparams.getTileSideSize()) throw new IllegalArgumentException("This message is yet to be determined. It could be another method in DeltaParameters");
    if(delta==0) return delta;

    dparams.offsetMovingCoordinate(delta);
    if(isInsideTile(dparams.getX(), dparams.getY())){
        int map = dparams.getMovingCoordinate() /dparams.getTileSideSize() ;
        int offset = dparams.getMovingCoordinate() - (map * dparams.getTileSideSize() );
        int maxDelta = 0;

        if(delta &lt; 0){
            maxDelta = delta + (dparams.getTileSideSize() - offset);
        }
        else if(delta &gt; 0){
            maxDelta = delta - offset;
        }

        return maxDelta;
    }
    return delta;
}


Now you could call getMaxDelta just like this

getMaxDelta(new DeltaXParameters(new Point(0, 0), tileMap), 5);


What I have done here is to take the advantage of polymorphism over DeltaParameters class. Which could not be a so straigthforward method to refactor is now proven to be in a quite simple fashion imho.