Find the nearest coin to be collected in a game

Question

I am creating a simple bot that collects coins in-game. The code runs under a game-loop so there are 60 ticks p/s

Here is the function that I use to check for the nearest coin (Game object) to be collected:

public GameObject getNearest(ID objectID) {

    LinkedList<GameObject> object = new LinkedList<GameObject>();
    for (int i = 0; i < handler.object.size(); i++) {
        GameObject temp = handler.object.get(i);
        if (temp.getId() == objectID) {
            object.add(temp);
        }
    }

    if (object.size() == 0)
        return null;

    else {
        GameObject nearest = object.get(0);
        for (int i = 0; i < object.size(); i++) {
            if (getDistance(this, object.get(i)) < getDistance(this, nearest)) {
                nearest = object.get(i);
            }
            object.remove(i);
        }
        return nearest;
    }
}

What this function would do is check for all the game objects, and add all the once that match the parameter in a list. It then will loop through the list and check for the nearest coin to the bot.

Is my function efficient regarding performance, speed, and memory usage? By calling the object.remove after every check, does it help? (If not, please mention why)

Is it okay that it calls the method 60 times p/s or I should limit it to execute once every 20 frames maybe? The thing is that I want it to be collecting the coins safely in the shortest time possible. Is there anything that I should take into consideration or something to improve in my code??

Answer 1

object should be plural, and should more clearly indicate what it contains. gameObjects would be preferable.

It's a good idea to always include curly braces, even when they're not required. It removes a possible source of later bugs.

It’s easier to read isEmpty() than size() == 0. You also don’t need the else statement, since the if returns.

If handler.object is a type that has an iterator() method, it would be better to use that than a for loop.

The performance of LinkedList is often worse than ArrayList. You should consider testing their relative performances.

You should code to interfaces, not implementations, where possible. List<GameObject> objects = … is preferable to LinkedList<GameObject> objects =.

If ID has a correct implementation of equals(), you should probably be using that. == asks if it’s the ID is the same object, not just if it has the same value.

In java, it’s preferable to only capitalize the first letter of an acronym, so objectId is preferred to objectID.

There’s probably something wrong with getDistance() - it should either be an instance method or you shouldn’t need to pass in this.

You don’t need to loop twice. Every time you find an object with a matching id, compare it’s distance to the current nearest object. Only keep the nearest. This will also save you from computing the distance twice.

As far as optimizing performance, you'll have to do some testing and see if it's working fast enough for you. Cutting out the one loop will make it closer to optimal, but we can't know how that will interact with the game.

If you were to apply all my suggestions, your code might look something like:

public GameObject getNearest(final ID objectId) {
    GameObject nearestObject = null;
    int distance = Integer.MAX_VALUE;

    for (int i = 0; i < this.handler.object.size(); i++) {
        final GameObject currentObject = this.handler.object.get(i);
        if (!currentObject.getId().equals(objectId)) {
            continue;
        }

        final int currentDistance = this.getDistance(this, currentObject);
        if (currentDistance < distance) {
            nearestObject = currentObject;
            distance = currentDistance;
        }
    }

    return nearestObject;
}