Dependency Injection into an Abstract Class

Question

Me and another developer have recently forked and taken over an abandoned, open-source project. I have also been reading articles and watching videos by Misko Hevery.

When I was reading through the project's code, I noticed a lot of no-argument constructors. And that the fields are being assigned via static calls instead of being assigned via the constructor parameters. When I saw this, I wanted to refactor the constructors to declare their dependencies explicitly.

This is simple to do.

However, the problem is that there are abstract classes with fields that need to be initialized. So what is the best way to do DI for parent classes?

I mean, the child class can ask for parameters (that it doesn't really need) in its constructor... only to pass it to the parent via super(arg1, arg2).

But this seems really bizarre to me: That the child is asking for something just to pass it to the parent. Is this really the way it's supposed to be done?

What about an abstract constructor (with necessary parameters) that forces the child to implement?

These are the two hierarchies that I'm wanting to refactor to use DI:

• class Arena extends class ArenaContainer extends abstract class AbstractAreaContainer

• class BAExecutor extends abstract class CustomCommandExecutor extends abstract class BaseExecutor

public class BAExecutor extends CustomCommandExecutor {
    Set<String> disabled = new HashSet<String>();

    final TeamController teamc;
    final EventController ec;
    final DuelController dc;
    final WatchController watchController;

    public BAExecutor() {
        super();
        this.ec = BattleArena.getEventController();
        this.teamc = BattleArena.getTeamController();
        this.dc = BattleArena.getDuelController();
        this.watchController = BattleArena.getSelf().getWatchController();
    }


public abstract class CustomCommandExecutor extends BaseExecutor{

    protected final BattleArenaController ac;
    protected final EventController ec;
    protected final ArenaEditor aec;

    protected CustomCommandExecutor(){
        super();
        this.ac = BattleArena.getBAController();
        this.ec = BattleArena.getEventController();
        this.aec = BattleArena.getArenaEditor();
    }

public abstract class BaseExecutor implements ArenaExecutor{
    public static final String version = "2.1.0";
    static final boolean DEBUG = false;
    private HashMap<String,TreeMap<Integer,MethodWrapper>> methods =
            new HashMap<String,TreeMap<Integer,MethodWrapper>>();
    private HashMap<String,Map<String,TreeMap<Integer,MethodWrapper>>> subCmdMethods =
            new HashMap<String,Map<String,TreeMap<Integer,MethodWrapper>>>();

    protected PriorityQueue<MethodWrapper> usage = new PriorityQueue<MethodWrapper>(2, new Comparator<MethodWrapper>(){
        @Override
        public int compare(MethodWrapper mw1, MethodWrapper mw2) {
            MCCommand cmd1 = mw1.getCommand();
            MCCommand cmd2 = mw2.getCommand();

            int c = new Float(mw1.getHelpOrder()).compareTo(mw2.getHelpOrder());
            if (c!=0) return c;
            c = new Integer(cmd1.order()).compareTo(cmd2.order());
            return c != 0 ? c : new Integer(cmd1.hashCode()).compareTo(cmd2.hashCode());
        }
    });
    static final String DEFAULT_CMD = "_dcmd_";

    /**
     * Custom arguments class so that we can return a modified arguments
     */
    public static class Arguments{
        public Object[] args;
    }

    protected static class MethodWrapper{
        public MethodWrapper(Object obj, Method method){
            this.obj = obj; this.method = method;
        }

        public Object obj; /// Object instance the method belongs to
        public Method method; /// Method
        public String usage;
        Float helpOrder = null;
        public MCCommand getCommand(){
            return this.method.getAnnotation(MCCommand.class);
        }
        public float getHelpOrder(){
            return helpOrder != null ?
                    helpOrder : this.method.getAnnotation(MCCommand.class).helpOrder();
        }
    }

    /**
     * When no arguments are supplied, no method is found
     * What to display when this happens
     * @param sender the sender
     */
    protected void showHelp(CommandSender sender, Command command){
        showHelp(sender,command,null);
    }

    protected void showHelp(CommandSender sender, Command command, String[] args){
        help(sender,command,args);
    }

    protected BaseExecutor(){
        addMethods(this, getClass().getMethods());
    }

Answer 1

BAExecutor

All variables can be instantiated in their declaration. You could shrink it to a half using

final TeamController teamc = BattleArena.getTeamController();

Also drop the empty super(), the JVM knows it has to call it.

However, when you want to use DI, this doesn't apply anymore. I can only recommend Lombok, allowing you to write

@RequiredArgsConstructor(onConstructor=@__(@Inject))
public class BAExecutor extends CustomCommandExecutor {
    Set<String> disabled = new HashSet<String>();

    final TeamController teamc;
    final EventController ec;
    final DuelController dc;
    final WatchController watchController;
}

It creates the constructor and adds the @Inject annotation. Pretty handy, wherever applicable.

BaseExecutor

static final boolean DEBUG = false;

DEBUG should probably be initialized using a corresponding entry from java.util.Properties, so you can change it without recompiling. It's used nowhere in the provided snippet.

private HashMap<String,TreeMap<Integer,MethodWrapper>> methods =
        new HashMap<String,TreeMap<Integer,MethodWrapper>>();

Use Java 7 diamonds or Guava's Maps.newHashMap(). Those repeating generics are a pain.

c = new Integer(cmd1.order()).compareTo(cmd2.order());

Boxing in order to use Integer#compareTo? Use Integer#compare instead.

    public Method method; /// Method

An outstanding comment.

Arguments

public static class Arguments{
    public Object[] args;
}

Unused class, missing space. However, I'd never use public mutable fields, especially no collections nor arrays. OTOH a class with all public getters and setters is not that much better, so it'd make all the boilerplate worth it (OTOH, Lombok's @Getter and @Setter make it trivial). I guess, not passing the arguments anywhere is best.

General

There's hardly anything to review, or I lost it when copying. To the question:

However, the problem is that there are abstract classes with fields that need to be initialized. So what is the best way to do DI for parent classes?

There's no nice solution. You really have to pass them all to the superclass. This is especially bad for me as I can't use my favorite Lombok annotation.

Sometimes, you may be lucky and see that the superclass actually needs nothing or maybe you can declare some abstract getters, so it can obtain what it needs from the subclasses.

Sometimes, you may find out that delegation is better than inheritance. You may end up with a simple parallel hierarchy, where the superclass has no fields. I'd need a more concrete example for this.