Should Guice Providers with expensive member instances as be annotated with @Singleton?

dependency-injection, guice, java, singleton

Solution

If your question is whether you should create scoped provider bindings, or if you should cache instances in your providers manually, then really, do not try to be smarter than Guice :) You really do not want to do anything more than just create your expensive object in `get()` method. Simple test case:

public class MyProvider implements Provider<String> {
    public String get() {
        System.out.println("Called MyProvider.get()");
        return "abcd";
    }
}

public class MyModule extends AbstractModule {
    protected void configure() {
        bind(String.class).toProvider(MyProvider.class).in(Singleton.class);
    }
}

Injector injector = Guice.createInjector(new MyModule());
String abcd1 = injector.getInstance(String.class);  // Prints "Called MyProvider.get()
String abcd2 = injector.getInstance(String.class);  // Prints nothing!
// Or, if you want, comment out above two lines and try the following:
Provider<String> abcdProvider = injector.getProvider(String.class);
abcdProvider.get();  // Prints "Called MyProvider.get()"
abcdProvider.get();  // Prints nothing

You see, because the message was printed only once, `MyProvider.get()` method was called only once too, exactly because `String` is bound in singleton scope.

The key concept to understand here is that providers and bindings are not separate entities. With every binding there is an associated provider (when you create plain bindings with `to()`, an implicit provider is created for you). This can easily be observed from `getProvider()` method signature - it accepts `Class<T>` or `Key<T>` for actual class you want to get, not for the provider you have bound. When you create a binding to specific provider, you do not configure this provider, you configure the binding. Guice is smart enough to take scopes into account even if you use explicit providers, so you just do not need to reinvent the wheel and roll out your own singleton.

If your question is specifically about usage of `@Singleton` annotation (as opposed to `bind()` DSL), then I don't know whether its presence on provider class gives any effect, but given that you should use `bind().toProvider()` to bind to this provider anyway, I don't think that it really matters. Just use `in()` method, it will certainly work.

Problem

Should Guice Providers be annotated with `@Singleton`? My justification: if the Provider is providing an object to other Singleton classes and the object itself is relatively expensive to create, then wouldn't it make sense to use a Singleton Provider that constructs the expensive object in its `@Inject`-marked constructor, store it as a member and just return that already-saved global variable in the getter? Something like this: ``` @Singleton public class MyProvider extends Provider<ExpensiveObject> { private ExpensiveObject obj; @Inject public MyProvider() { /* Create the expensive object here, set it to this.obj */ } @Override public ExpensiveObject get() { return obj; } } ``` Update Let me clarify a little bit more here. This is not about whether I should be using `@Singleton` or `.in(Singleton.class)`. This has to do more with the "caching" of the created object. Let's say that object creation required multiple RPCs to complete, such as deserializing JSON or making HTTP requests. This could take quite some time. If I am going to use this Provider to inject into classes multiple times, then doesn't it make sense to only create such an object once? Also note that I must be able to use a Provider because I need to be able to inject into the Provider.

Original source