Service Locator Pattern vs AssistedInject

700 views
Skip to first unread message

Gili

unread,
Nov 7, 2012, 3:50:39 PM11/7/12
to google...@googlegroups.com
Hi,

I've been using Guice over the past couple of years and it worked quite well for me. However, every once in a while I run across a recurring problem and I'd like to get your feedback on it. The problem arises when I run across a constructor that requires a mix of Guice-injected and user-supplied values. For example:

  public MyService(UriInfo uriInfo, String userName)

This is precisely the use-case AssistedInject is meant to address but the more I run into this code, the more I am leaning towards using the Service Locator pattern [1] instead. AssistedInject requires me to construct and bind one factory per class. This means that every time I add/remove a class, I need to add/remove a binding. It means every time I add/remove user-supplied parameters, I need to also add/remove parameters from the factory. Initially it didn't seem like a big deal but time I've questioned the cost/benefit of this approach. The resulting code is harder to read and maintain, and for what? Nowadays I replace:

  @Inject
  public MyService(First first, Second second, Third third, @Assisted String userName)

with

  @Inject
  public MyService(Injector injector, String userName)
  {
    this.first = injector.getInstance(First.class);
    this.second = injector.getInstance(Second.class);
    this.third = injector.getInstance(Third.class);
  }

and simply inject classes on-demand using injector. Essentially I'm using Guice as a Service Locator. The way I see it:
  • The classes/constructors are a lot easier to maintain.
  • No need to enumerate the injected parameters in the Javadoc. I've always found it annoying to have the Javadoc cluttered with parameters the user doesn't really care about (besides the fact it was a lot of work to keep such documentation up-to-date).
  • This still implements the inversion of control pattern so the code is easily testable.
  • The dependencies might not be listed as constructor parameters, but they still visible near the top of the constructor.
  • To clarify, I only advocate this approach when mixing injected and user-supplied parameters. I still use normal constructor injection when all parameters come from Guice.
So really, what's the harm? I'd love to get your feedback on this.

Thanks,
Gili

Christian Gruber

unread,
Nov 7, 2012, 4:06:14 PM11/7/12
to google...@googlegroups.com
Well, for starters, you've now created a dependency on the injector,
which is valid but I find makes code substantially more confusing in the
long-run. It also means that we could never EVER analyze your code
statically to determine if your wiring was correct (that is, all
dependencies are bound) because you've internalized the logic of the
dependency fulfillment into a code-path instead of a declaration.

Also, you can't see the dependencies from without - say, from method
signatures or otherwise, so it's harder to reason things through. You
can always look at the code, but the constructor signature is now
meaningless - it doesn't semantically include the dependencies - it
includes a piece of infrastructure.

You're right - there's a bit more work to do from the outside with
assisted injection in terms of maintaining dependency lists. But the
clarity in the code I find to be valuable.

In practice, I don't find it that difficult to keep these things clear.

public class Foo {

//private variables

// factory interface near constructor.
public interface Factory {
Foo create(Consumable someValue);
}

Foo(Dependency dep, OtherDependency dep2, @Assisted Consumable
someValue) {
...
}

}

This way, if I need to change that list, I change the two nearly beside
each other - easy to see.

The other problem is that with assisted injection, you now can separate
different KINDS of injected things - collaborating services,
configuration values, consumables, and pass them around differently.
Consumables (value types, local differences) have quite different life
cycles from services, etc. So if you do what you're suggesting, you
absolutely start to need to play scope games. Because if you have
values that aren't just created once for each request being part of the
object's lifecycle, you now have to create narrower and narrower @Scopes
in order to support these shorter lifetimes.

Sometimes things really just need to be on the call-stack. And having a
factory break as part of a factory call is a far clearer error (and
stack-trace) than a deep failure of injection. They might be
systemically equivalent, but their meaning to the coder is different,
and finding the errors later are quite different.

Generally, anything you might want to use Guice as a factory for, I tend
to recommend using assisted injection, because it creates separate,
traceable factories for each type. Yes, it's "more work" in one sense -
but I think it's a better organization of code.

Injecting the injector is something that is possible, and sometimes
necessary to support legacy code. But I think the problems outweigh the
ease you think it'll buy you - especially for down-stream maintainers.

Christian
> - The classes/constructors are a lot easier to maintain.
> - No need to enumerate the injected parameters in the Javadoc. I've
> always found it annoying to have the Javadoc cluttered with parameters
> the
> user doesn't really care about (besides the fact it was a lot of work
> to
> keep such documentation up-to-date).
> - This still implements the inversion of control pattern so the code
> is
> easily testable.
> - The dependencies might not be listed as constructor parameters, but
> they still visible near the top of the constructor.
> - To clarify, I only advocate this approach when mixing injected and
> user-supplied parameters. I still use normal constructor injection
> when all
> parameters come from Guice.
>
> So really, what's the harm? I'd love to get your feedback on this.
>
> Thanks,
> Gili
>
> [1]
> http://martinfowler.com/articles/injection.html#UsingAServiceLocator
>
> --
> You received this message because you are subscribed to the Google
> Groups "google-guice" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/google-guice/-/HSTXZ7On-acJ.
> To post to this group, send email to google...@googlegroups.com.
> To unsubscribe from this group, send email to
> google-guice...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/google-guice?hl=en.

Alex opn

unread,
Nov 7, 2012, 4:10:08 PM11/7/12
to google...@googlegroups.com
factories inside the classes itself near constructor! clever :)!


To unsubscribe from this group, send email to google-guice+unsubscribe@googlegroups.com.

For more options, visit this group at http://groups.google.com/group/google-guice?hl=en.
--
You received this message because you are subscribed to the Google Groups "google-guice" group.
To post to this group, send email to google...@googlegroups.com.
To unsubscribe from this group, send email to google-guice+unsubscribe@googlegroups.com.

Christian Gruber

unread,
Nov 7, 2012, 4:14:43 PM11/7/12
to google...@googlegroups.com
Well, I actually put mine at the bottom of the class, personally. But
to ease clarity, I could see putting them close to the constructor.
But only if they're factory interfaces. IF you have a concrete factory
(as in, you're not using assisted injection but hand-coding factories)
then I think they'd clutter the top of a class.

Your style will vary, my point primarily being that you keep the things
that change together close together by convention. Makes life easier.

Christian.
>>> this.first = injector.getInstance(First.**class);
>>> this.second = injector.getInstance(Second.**class);
>>> this.third = injector.getInstance(Third.**class);
>>> }
>>>
>>> and simply inject classes on-demand using injector. Essentially I'm
>>> using
>>> Guice as a Service Locator. The way I see it:
>>>
>>> - The classes/constructors are a lot easier to maintain.
>>> - No need to enumerate the injected parameters in the Javadoc. I've
>>>
>>> always found it annoying to have the Javadoc cluttered with
>>> parameters the
>>> user doesn't really care about (besides the fact it was a lot of
>>> work to
>>> keep such documentation up-to-date).
>>> - This still implements the inversion of control pattern so the code
>>> is
>>> easily testable.
>>> - The dependencies might not be listed as constructor parameters,
>>> but
>>>
>>> they still visible near the top of the constructor.
>>> - To clarify, I only advocate this approach when mixing injected and
>>>
>>> user-supplied parameters. I still use normal constructor injection
>>> when
>>> all
>>> parameters come from Guice.
>>>
>>> So really, what's the harm? I'd love to get your feedback on this.
>>>
>>> Thanks,
>>> Gili
>>>
>>> [1] http://martinfowler.com/**articles/injection.html#**
>>> UsingAServiceLocator<http://martinfowler.com/articles/injection.html#UsingAServiceLocator>
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups
>>> "google-guice" group.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/**
>>> msg/google-guice/-/HSTXZ7On-**acJ<https://groups.google.com/d/msg/google-guice/-/HSTXZ7On-acJ>
>>> .
>>> To post to this group, send email to google...@googlegroups.com.
>>> To unsubscribe from this group, send email to
>>> google-guice+unsubscribe@**
>>> googlegroups.com <google-guice%2Bunsu...@googlegroups.com>.
>>> For more options, visit this group at http://groups.google.com/**
>>> group/google-guice?hl=en<http://groups.google.com/group/google-guice?hl=en>
>>> .
>>>
>>
>> --
>> You received this message because you are subscribed to the Google
>> Groups
>> "google-guice" group.
>> To post to this group, send email to google...@googlegroups.com.
>> To unsubscribe from this group, send email to
>> google-guice+unsubscribe@**
>> googlegroups.com <google-guice%2Bunsu...@googlegroups.com>.
>> For more options, visit this group at http://groups.google.com/**
>> group/google-guice?hl=en<http://groups.google.com/group/google-guice?hl=en>
>> .
>>
>>
>
> --
> You received this message because you are subscribed to the Google
> Groups "google-guice" group.
> To post to this group, send email to google...@googlegroups.com.
> To unsubscribe from this group, send email to
> google-guice...@googlegroups.com.

cowwoc

unread,
Nov 7, 2012, 4:53:38 PM11/7/12
to google...@googlegroups.com
Hi Christian,

    Let's work through these one by one.
  • We lose the ability to static analysis the code to determine if the wiring is correct.
    • I've never heard of using static analysis to checking wiring. Can you please point me to an example?
  • We can't see dependencies from method signatures.
    • True, but I argue that users shouldn't be aware of these dependencies.
    • We're asking users to pass in what amount to pseudo-constants. Imagine if we used inversion of control to pass in Math.PI. Would users be happy to know that a class depends on the value of PI? Who cares? :)
    • Typically, injected types are bound once and users never see/hear about it ever again. Often, inject values come from external frameworks (e.g. Jersey). This stuff "just works" 99% of the time.
    • I argue users should only see parameters that are most likely to concern them. When I construct a type where some parameters are injected and some are supplied by the user, I only never pay attention to the injected types. Should I?
  • This approach makes it harder to maintain dependency lists.
    • I suppose you could generate dependency lists from your Module(s) or invoke the Guice SPIs.
    • Though, I argue that the entire point of Guice is to hide such dependency lists from us.
    • If we really cared to see them in their glory, we'd pass all transitive dependencies into the top-level class. That way it would be clear that we really depend on all of these types.
    • Instead, Guice hides transitive dependencies from us for the sake of clarity (as it should).
  • Place AssistedInject factories near the constructor.
    • I agree this helps but like you said it ends up cluttering the file so I also ended up moving it to the bottom.
    • And remember, the module/bindings still reside in a separate file, so you're still going to have a harder time keeping the two synchronized.
  • You brought up an interesting point about being able to inject different kids of injected things (e.g. collaborating services, etc) but I didn't really understand what you meant. Can you please provide a more concrete example?
Thanks,
Gili

Christian Gruber

unread,
Nov 7, 2012, 5:38:56 PM11/7/12
to google...@googlegroups.com

Inline:

On 7 Nov 2012, at 16:53, cowwoc wrote:

Hi Christian,

Let's work through these one by one.

  • We lose the ability to static analysis the code to determine if the wiring is correct. o I've never heard of using static analysis to checking wiring. Can you please point me to an example?

Guice doesn't do it now, but Dagger does, and Guice ultimately will, if you limit your use to patterns that are not inherently hard (computationally) to analyze.

  • We can't see dependencies from method signatures. o True, but I argue that users shouldn't be aware of these dependencies.

This is complex… it depends on the kind of object this is. Typically objects you would want to create from factories based on data you have in your local stack frame already (i.e., locals, parameters, etc.) aren't ones that you want to hide the parameters of.

You're using the word dependency too broadly… what KIND of dependencies are you talking about. In some concrete cases I might agree with you. In others I would not.

o We're asking users to pass in what amount to pseudo-constants.


Imagine if we used inversion of control to pass in Math.PI.
Would users be happy to know that a class depends on the value
of PI? Who cares? :)

Can you please pick a non-absurd example of this? I would not use dependency injection at all to supply Math.PI.

they're not pseudo-constants… they're scoped. Math.PI is global and stateless data. That's quite different than a service object. It's also different than a piece of context that can't be a constant, because it's not universal (say, a "current user"), but is effectively like a constant for that object's perspective. I don't mind injecting these, in carefully controlled circumstances. But usually I find that the benefits don't outweigh the costs on code clarity. This is subjective.

o Typically, injected types are bound once and users never


see/hear about it ever again. Often, inject values come from
external frameworks (e.g. Jersey). This stuff "just works" 99%
of the time.

I'm not sure how this is relevant to service vs. factory method. IF you don't want the calling clients to see it, don't include it in the service interface, and just inject it normally. You don't have to change a factory method signature for non-assisted constructor parameter changes.

o I argue users should only see parameters that are most likely to


concern them. When I construct a type where some parameters are
injected and some are supplied by the user, I only never pay
attention to the injected types. Should I?

assisted-inject lets you mix this… actually, the end-clients should only pay attention to the "assisted" parameters. The other injected dependencies are entirely invisible to calling code, as it's injected by guice.

  • This approach makes it harder to maintain dependency lists. o I suppose you could generate dependency lists from your Module(s) or invoke the Guice SPIs.

Possibly - I'd need to see this in practice to decide if it's worth the payoff. My instinct says no, but I have things I'm used-to.

o Though, I argue that the entire point of Guice is to hide such
dependency lists from us.

Not all dependency lists, and not in all places at all times. Again - you're using the one word dependency to refer to many kinds of things that have different purposes, uses, structures, and life cycles. It's important to define these kinds of things in your code to determine if injecting them automatically, manually, or not at all is appropriate.

o If we really cared to see them in their glory, we'd pass all


transitive dependencies into the top-level class. That way it
would be clear that we really depend on all of these types.

Except it doesn't depend on these types in any way that it cares about. It implicitly does, but the point of Guice-style DI is to separate explicit from implicit dependencies. If you did what you describe, you force all classes to depend on all classes, and altering behavior elsewhere in the system would require changing it in all places, rather than only behind the wall of encapsulations.

o Instead, Guice hides transitive dependencies from us for the


sake of clarity (as it should).

And other benefits to this hiding, but yes, agreed.

  • Place AssistedInject factories near the constructor. o I agree this helps but like you said it ends up cluttering the file so I also ended up moving it to the bottom.

If it's just an interface, I don't think it would clutter the file much. If it's a class with method bodies, it would. I guess I just treat it as two places I have to look. Sadly, there isn't (yet) a refactoring plugin that can instruct IDEA/Eclipse to be aware of @Assisted properties and their factory companions, so you can change it once. I still maintain that this is a small cost for better cognitive result in the resulting code.

o And remember, the module/bindings still reside in a separate


file, so you're still going to have a harder time keeping the
two synchronized.

No - if you're re-organizing the dependencies, but no new things are bound, then you don't have to change the bindings. The bindings aren't the dependency graph, they're the manifest of what participates in the system. If the types are all registered (@Provides or bind().to()) then you're all set.

  • You brought up an interesting point about being able to inject different kids of injected things (e.g. collaborating services, etc) but I didn't really understand what you meant. Can you please provide a more concrete example?

Sure. These are a bit cerebral, but are quite familiar in most web apps in most companies.

  • Services: - global and expensive (often), providing facilities for other classes

    • A UserService is a global service. A DatabaseManager is a global service At Google we have Gaia and Megastore equivalents, for example.
  • Factories: - a kind of service that creates types that are frequently created.

    • A UserService that produces a User based on a lookup of data could be considered a Factory
    • A FooService, where Foos need to be created often, and may have some injectable properties and some local state consumed to create it.
  • Configuration/Context: A value type or complex object that is used to provide context for services and other logic, but is (from its dependents' perspective) largely immutable. A pseudo-constant, as you call it.

    • A User could be context/configuration if it's global from the perspective of a given operation. If that operation is encapsulated in an object, then a User could be its global context. But only if it's playing a role such as a "current user". It's not configuration if it's in a list of Users being acted upon. In that case it's a value type.
    • A string that's annotated or the value of a key-value pair that has some global meaning, such as the connection string to a database, etc.
  • Value Type - Data. Primitive types, primitive holders/wrappers, complex objects which represent the data of a program, collections of such, etc.

    • A User in some contexts is a value type.
    • Primitive java types (ints, floats, strings) when created and consumed in the normal flow of business logic.
    • A Collection operated upon by business logic.

Assisted Inject is highly useful when you have composite value types - complex objects that need some data in their creation - data that makes them unique (A User's userId) - and some services they rely on to do their job, say, a LoggingService, or some other internals.

Whether something fits these categories depends on their usage. Even something like a User can fit multiple roles in different contexts. So it's not hard and fast. But things like assisted inject give you the opportunity to start segregating these roles, and seeing how things are used based on their patterns of use is a helpful signal in understanding the code flow.

This is, as an aside, why I'm often uncomfortable with people writing too much code that injects value types as context. It can be powerful, but it can also confuse things, especially if that value type is context here, but data over there. It can lead people to need to create artificial scopes in Guice just to handle the fact that they structured their code to treat a value type as context, always.

Hope that helps.

Chirstian.

Reply all
Reply to author
Forward
0 new messages