Collision detection implementation

Question

I'm working on a clone of Hunter Story. You shoot monsters with your bow. Recently I implemented collision detection, this is all working correctly, and I don't think the code needs any dramatic optimisations. The only thing I'm a bit worried about is how exactly I implemented it. I'm not sure if it complies with OOP standards.

The relevant code:

Level:

public class Level extends Entity
{
    public static ArrayList<Monster> onScreenMonsters = new ArrayList<Monster>();    //holds all monsters on the screen
    public static ArrayList<Arrow> activeArrows = new ArrayList<Arrow>();   //holds all arrows on the screen

    private void removeMonsters()
    {
        for (int i = 0; i < onScreenMonsters.size(); i++) 
        {
            Monster m = onScreenMonsters.get(i);
            if (m != null && m.getX() < 0)
            {
                onScreenMonsters.remove(m);    //removes monsters that are out of bounds.
            }

            if (!m.isAlive())             //set to false when monster has been hit (Will be modified once I properly implement damage, etc.)
            {
                onScreenMonsters.remove(m);
                monstersLeft--;
            }
        }
    }

    public static void removeInactiveArrows()
    {
        for (int i = 0; i < Level.activeArrows.size(); i++) 
        {
            Arrow a = Level.activeArrows.get(i);
            if (a != null && a.getY() > 720 - 130)
            {
                Level.activeArrows.remove(a);    //Remove arrows that are out of bounds
            }

            if (!a.isActive())
                Level.activeArrows.remove(a);    //Remove all arrows that are inactive (have hit a target)
        }
    }

    private void CheckCollision(Arrow a, Monster m)
    {
        boolean collision = true;
        if (a.hasHit())
            collision = false;
        else if (a.getX2() < m.getX())
            collision = false;
        else if (a.getY2() < m.getY())
            collision = false;
        else if (a.getY() > m.getY2())
            collision = false;
        else if (a.getX() > m.getX2())
            collision = false;

        if (collision)
        {
            a.setHit(true);    //prevents arrows from hitting multiple targets.
            m.onCollision(a);
            a.onCollision(m);
        }
    }
}

Monster:

public class Monster extends Entity 
{
    private boolean alive = true;

    public boolean isAlive() 
    {
        return alive;
    }


    public void setAlive(boolean alive) 
    {
        this.alive = alive;
    }

         public void onCollision(Arrow a)    //a is not used at the moment, but I will need it later to determine damage, etc.
    {
        setAlive(false);
    }
}

Arrow:

public class Arrow extends Entity
{
    private boolean active = true;
    private boolean hit = false;

    public boolean isActive() {
        return active;
    }

    public void setActive(boolean active) {
        this.active = active;
    }

    public boolean hasHit() {
        return hit;
    }


    public void setHit(boolean hit) {
        this.hit = hit;
    }       

    public void onCollision(Monster m)    //m is not used at the moment, but I might need it later.
    {
        setActive(false);
        setHit(true);
    }
}

Entity Base Class (gives the other classes a position and a size, etc):

public abstract class Entity implements Updateable, Renderable
{
    private float x;
    private float y;
    private float width;
    private float height;


    public float getX()
    {
        return x;
    }

    public void setX(float x)
    {
        this.x = x;
    }

    public float getY()
    {
        return y;
    }

    public void setY(float y)
    {
        this.y = y;
    }

    public float getX2() {
        return x + width;
    }


    public float getY2() {
        return y + height;
    }
}

(Keep in mind that I've left out all irrelevant code. If you want to see the sources, follow the link below).

It's quite a bit of code, but I'm not looking for code optimisations. I only need to know whether this is a good OOP implementation. Would I be better off having a separate CollisionDetector class, where I pass my colliders (Arrow and Monster) in? Should I implement the collision checking in the colliders themselves? Or is there perhaps something I haven't thought of?

If you want to see the full project, check here. It's made in Java with the Slick2D framework.

Answer 1

You could add an abstract onCollision() method to Entity and move the CheckCollision method to Entity so it would be:

arrow.checkCollision(monster)

That is all that I can think of, it looks pretty solid to me.