Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to DRY these block of code in Java?

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?

like image 441
Albert Gao Avatar asked May 09 '16 05:05

Albert Gao


2 Answers

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>

  1. 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.

like image 93
Rogue Avatar answered Sep 28 '22 17:09

Rogue


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 {
}
}
like image 22
Hasan Avatar answered Sep 28 '22 19:09

Hasan