Guice assisted inject overuse alternatives, dependency injection vs non-DI constructors

Question

The main reasons I like passing in runtime dependencys in constructors are:

1. It makes the dependency required
2. It provides a central place to set instance variables
3. Setting the dependency as an instance variable prevents you from having to pass it around from method to method within the class or pass it in twice or more to two or more public methods

This has led me to use a lot of Assisted Injects when using Guice. This creates extra code compared to not using DI so reading things like this: How exactly is Assisted-inject suppose to be use to be useful?

It seems like most people don't pass the runtime(derived, not available at startup) dependencies in constructors using assisted inject, and instead pass them in individual methods. Thats fine for the simple class given in the above stackoverflow post where there is only one method that relies on the dependency:

public class SomeClass {
    @Inject
    SomeClass(...) {
        ...
    }

    public void doWork(int s) { /* use s */ ... }
}

But what if the class has many methods that use the dependency? Do you pass it from the public method to private methods and require it passed in on all public methods? For example:

public class SomeClass {
    @Inject
    SomeClass(...) {
        ...
    }

    public void doWork(int s) { 
     /*some code */
     someOtherMethod(s);
     anotherMethod(s);  

    }
    //any private method that needs it gets it passed in as a param
    private void someOtherMethod(int s)...
    private void anotherMethod(int s)...

    //require it passed in all public methods that need it
    public void anotherPublic(int s){
      someOtherMethod(s);
    }
}

As opposed to using constructors this adds a bit of extra code as seen here:

public class SomeClass {
    private int s;
    SomeClass(int s) {
        this.s = s;
    }

    public void doWork() { 
     someOtherMethod();
     anotherMethod();  

    }
    private void someOtherMethod()...
    private void anotherMethod()...

    public void anotherPublic(){}
}

Or would you set the instance var from the service method like this?

public class SomeClass {
    Integer s;
    @Inject
    SomeClass(...) {
        ...
    }

    public void doWork(Integer s) { 
     /***set instance var this time***/
      this.s = s;
     someOtherMethod();
     anotherMethod();  

    }
    private void someOtherMethod()...
    private void anotherMethod()...

    public void anotherPublicMethod(){
     if(s==null){ //check if s was set already
         throw new IllegalStateException();
      }else{
      /* do something else */
      }
    }


}

Or would you pass the dependency into the other public method as a param and set the instance var there as well? For Example:

public class SomeClass {
    @Inject
    SomeClass(...) {
        ...
    }

    public void doWork(Integer s) { 
     /***set instance var this time***/
      this.s = s;
     someOtherMethod();
     anotherMethod();  

    }
    private void someOtherMethod()...
    private void anotherMethod()...

    public void anotherPublicMethod(Integer s){
       this.s = s;
      /* do something else */
    }


}

So I think passing the param from method to method or throwing illegal state exceptions to check for it isn't ideal compared to using normal constructors, but obviously there are advantages/disadvantages to any framework/pattern.

If I am just not separating my objects in the ideal way, please let me know some guidelines you use, ie "I only use one public method per service class, see this book or post about it:.." .

What do you guys do in the above situations?

Answer 1

You nailed down some great reasons to use assisted injection in your question: It ensures that the object instances only ever exist in a fully-initialized state, keeps your dependencies together, and frees the object's public interface from requiring a predictable parameter in every method.

I don't really have any alternatives to add, other than the ones you mentioned:

• Adding a setter method for that dependency, probably requiring IllegalStateException checks or a good default value
• Creating an initialize(int s) pseudoconstructor method with the same IllegalStateException checks
• Taking in the parameter in individual methods
• Replacing the FactoryModuleBuilder boilerplate with a custom factory, thereby creating more extra boilerplate you're trying to avoid

My favorites are the two you seem to be deciding between--assisted injection or taking the parameter in every method--mostly because they both keep the object in a predictable, usable state at all times. My decision between them rests on what kind of state the object should carry, whether that state is mutable, and how I want to control instances. For Car.licensePlateNumber, the license plate number may vary with the car instance; each car has one license plate number that (in this example) never varies, and the car isn't valid without it, so it should be a constructor argument. Conversely, Repository<T> may frequently take in the same T instance in all of its methods, but a Repository is still a Repository no matter which instance you pass in, and you may want the freedom to reuse that instance without creating a new one for each T (as you may have to do with assisted injection). Both designs are valid, and each one is optimal for a certain set of cases.

Remember that there shouldn't really be that much extra code required for assisted injection:

/** In module: install(new FactoryModuleBuilder().build(SomeClass.Factory.class)); */
public class SomeClass {
  public interface Factory {
    SomeClass create(int s);
  }

  private final int s;

  @Inject
  SomeClass(/* ..., */ @Assisted int s) {
    this.s = s;
  }

  public void doWork() { /* ... */ }
}