How to DRY these block of code in Java?

Question

Caller:

switch (type){
            case "creature":
                Creature returnActor2 = getNextCreature();
                boolean isEat2 = actOnNearby(getRightChromosome(Config.HardCode.creature), returnActor2.getLocation());
                if (isEat2) {
                    actOnCreature(returnActor2);
                }
                break;
            case "monster":
                Monster returnActor3 = getNextMonster();
                boolean isEat3 = actOnNearby(getRightChromosome(Config.HardCode.monster), returnActor3.getLocation());
                if (isEat3) {
                    actOnMonster(returnActor3);
                }
                break;
}

It will call the following 2 methods:

    private Monster getNextMonster() {
        ArrayList<Actor> nearbyActors = getActors();
        Monster mine = new Monster();
        for (Actor a : nearbyActors) {
            if (a instanceof Monster) {
                mine = (Monster) a;
            }
        }
        return mine;
    }


private Creature getNextCreature() {
    ArrayList<Actor> nearbyActors = getActors();
    Creature mine = new Creature();
    for (Actor a : nearbyActors) {
        if (a instanceof Creature) {
            mine = (Creature) a;
        }
    }
    return mine;
}

The question
As you can see, the getNextXXXXX() method are pretty the same, just return different object, the logic is same, how to DRY? the actOnXXXX() seems falls in the DRY category as well, but it all about the same, use same logic against different object. How to solve this?

Answer 1

Make it accept a classtype:

private <T> T getNext(Class<T> type) {
    for (Actor a : getActors()) {
        if (type.isAssignableFrom(a.getClass())) {
            return (T) a;
        }
    }
    return null; //or type.newInstance(); if you want a guaranteed object, but this restricts your constructor.
}

Or with Java 8:

private <T> T getNext(Class<T> type) {
    return (T) getActors().stream()
                .filter(a -> type.isAssignableFrom(a.getClass()))
                .findFirst().orElse(null);
}

But the usage is the same:

Monster next = getNext(Monster.class);

Breaking down the problem, you know two categories of things:

What you need:

• A next object of t type.
• A way of determining if an object is t type

What you have:

• The type t you want
• A collection of objects, one of which might be t type
• A new object via a no-args constructor (or null) if none are there

Additionally, the only variance between all these methods is one thing: Which type it is. So we literally "make that a variable", and as such it becomes a method parameter.

Breaking this down we simply need to organize the code in a manner which accomplishes this:

method: //receives a "type" as a parameter
    iterate the list of possible `t`s //our list of objects
        if some_t == type //our comparison, previously `a instanceof Type`
            return some_t //our result is found
    return null //or a new object, but essentially our "default"

The only primary differences here were:

1. Replacing some_t instanceof Type with type.isAssignableFrom(some_t.getClass())

Reason being here that this is simply how you determine this when using Class<T>

2. Our default can either be null or a new object

Making the object dynamically via reflection restricts your options and has exceptions to deal with. Returning null or an empty Optional<T> would help signify that you did not have a result, and the caller can act accordingly. You could potentially also just pass the default object itself, and then go back to the instanceof check.

Asking yourself this same hypothesis of "what do I need, and what can I provide/have", will help you map out breaking down the problem into smaller steps, and solving the larger puzzle.

Answer 2

I think what you want is to combine getNextMonster and getNextCreature because they have repeated code.

The best thing to do here is to write a generic method that does this:

private <T extends Actor> T getNextActor(T newActor) {
    ArrayList<Actor> nearbyActors = getActors();
    T mine = newActor;
    for (Actor a : nearbyActors) {
        if (a instanceof T) {
            mine = (T) a;
        }
    }
    return mine;
}

And you can call it like this:

// This is equivalent to calling getNextCreature()
getNextActor(new Creature());

// This is equivalent to calling getNextMonster()
getNextActor(new Monster());

Let me explain the code.

The new method returns a type of Actor. You tell it what kind of actor you want by passing the argument. The argument is necessary because you cannot just initialize a generic type argument like this:

new T();

Because the parameterless constructor might not be available. So that's the job of the caller.

_{I don't really know what I'm talking about...}

This method has the following advantages:

• It reduces repeated code
• It is flexible - when you want to add another method called getNextXXX (where XXX is a subclass of Actor), you don't need to. Just call getNextActor(new XXX())!
• It increases maintainability - if you want to change the implementation of getNextXXX, you can just change one method instead of 2.

Answer 3

I think, there is a confusion in your code and logic. FOr example, if you need to iterate on list, you dont need to create a new object. That is, in the following code snippet, "new Monster()" doesn't need to be written

Monster mine = null; // new Monster();
for (Actor a : nearbyActors) {
    if (a instanceof Monster) {
        mine = (Monster) a;
    }
}

Anyway, the answer is the "Type Inference in Java." https://docs.oracle.com/javase/tutorial/java/generics/genTypeInference.html

The answer to your question is

package __TypeInference;

import java.util.ArrayList;
import java.util.List;

public class Main {

public static void main(String[] args) {
    new Main().doLogic();
}

private void doLogic() {
    List<Actor> nearbyActors = getActors();
    for (Actor actor : nearbyActors) {
        // do with the next actor
        System.out.println(actor.toString());
    }
}

private List<Actor> getActors() {
    List<Actor> actors = new ArrayList<Actor>();
    actors.add(new Monster());
    actors.add(new Creature());
    actors.add(new Monster());
    actors.add(new Creature());
    return actors;
}

class Monster extends Actor {
    @Override
    public String toString() {
        return "Monster";
    }
}

class Creature extends Actor {
    @Override
    public String toString() {
        return "Creatue";
    }
}

class Actor {
}
}