Java constructor with Guice injected fields along with non-injected fields

Question

I have a class which has a constructor where all the arguments are injected by GUICE.

Public class Order {

    private final ClassOne classOneObj;
    private final ClassTwo classTwoObj;

    @Inject
    public order(ClassOne classOneObj, ClassTwo classTwoObj){
    this.classOneObj = classOneObj;
    this.classTwoObj = classTwoObj;
    }
}

Now, I want to add one more field(say, int status)variable which can't be injected.

Is it a good practice to create an object first with all the injected parameters and then set the new field which can't be injected with a setter method?

I came up with another approach where I created a factory class as given below:

public class OrderFactory {

    private final ClassOne classOneObj;
    private final ClassTwo classTwoObj;

    @Inject
    public order(ClassOne classOneObj, ClassTwo classTwoObj){
    this.classOneObj = classOneObj;
    this.classTwoObj = classTwoObj;
    }

   //getter methods for all the above variables

    public  ClassOne getclassOneObj(){
          return classOneObj;
    }
    ....

    public Order createOrder(int status) {
        return new Order(status, classOneObj, classTwoObj);
    }
}

Then the new Order class will look like

public class Order {

    int status
    private final ClassOne classOneObj;
    private final ClassTwo classTwoObj;


    public order(int status, ClassOne classOneObj, ClassTwo classTwoObj){
    this.status = status
    this.classOneObj = classOneObj;
    this.classTwoObj = classTwoObj;
    }

    //getter methods for all these member variables
}

Now to create the order object I will first create an OrderFactory object and then using the "createOrder" method I will create the Order object.

I am ending up with writing boilerplate code. Not sure if this is a good practice. Can anybody suggest on this if this approach is correct or there is any better approach for this problem?

I read in Google Guice and found there is a feature @Assisted for assisted injection for such cases. But I found that complex and couldn't convince myself whether I should go with that in my case.

Thanks in advance for any suggestion or guidance.

Echoing iluxa, your manual approach to factories is fine, though you can make assisted injection work in a much shorter fashion. The only thing I would change about your manual injection is to inject a `Provder` and `Provider` instead, if you want to create many different Orders with the same OrderFactory. Otherwise, it looks great. — Jeff Bowman, Nov 02 '13 at 01:55

score 4 · Answer 1 · answered Nov 02 '13 at 00:14

Your factory approach is excellent. Please don't use the setters: if the field can be made immutable, it should be immutable, whether or not it makes it "convenient" to instantiate.

Another approach you can take is Assisted Injection, which solves this exact problem. With that, you only define the factory interface, and its implementation is magically given to you by Guice:

class Order {
  interface Factory {
    Order create(Status status);
  }

  @AssistedInject Order(
      ClassOne one, 
      ClassTwo two, 
      @Assisted Status status) {
  }
}

Module code:

bind(Order.Factory.class).toProvider(
    FactoryProvider.newFactory(Order.Factory.class, Order.class));

Then the clients inject Factory and use it just like they do in your example.

score -1 · Answer 2 · answered Nov 01 '13 at 22:49

-1

You're typically going to inject things that take some amount of effort to construct. If you're just injecting an int field, you'd be better off just calling a setter method on the object (that has some of it's more complex dependencies injected). Also, if a fields changes frequently, as implied by a field called "status", then it's also not a good candidate for injection.

answered Nov 01 '13 at 22:49

Alper Akture

2,445
1
30
44

1

Please avoid setters whenever possible. If there's any chance to keep the object immutable and all its fields `final`, do that. – iluxa Nov 02 '13 at 00:15
1

+1 to iluxa. Immutable objects have [a lot of benefits](http://stackoverflow.com/a/214718/1426891). We all agree that injecting `status` is a bad idea, but the OP's question was about letting injection coexist with _non-injected_ constructor arguments, for which there are many valid use-cases. – Jeff Bowman Nov 02 '13 at 01:51
Yeah, working for Google teaches you a thing or two about guice doesn't it... :-) – iluxa Nov 03 '13 at 13:32
I agree immutability especially with respect to multi threading is best. In his use case, a field called "status" seems to indicate volatility, maybe it's not (especially if he's providing a factory for it). If that field never changes, sure inject it. If it does, you want to create a new one each time? int assignment is atomic, make it volatile and you're good to go. – Alper Akture Nov 04 '13 at 23:29

Java constructor with Guice injected fields along with non-injected fields

2 Answers2