Over a million developers have joined DZone.

Why @Inject is a Bad Idea

· Java Zone

Discover how AppDynamics steps in to upgrade your performance game and prevent your enterprise from these top 10 Java performance problems, brought to you in partnership with AppDynamics.

Recently the JSR for standardizing injection annotations was announced. I think the proposal is going in the wrong direction, and would like to outline why in this post.

One of the main insights we have had so far in the Qi4j project is that if "anything can mean anything" then that is the same as "nothing means anything". Specifically, if all you need is a POJO to implement entities, values and services, then it is really really hard to ensure that any meaningful semantics is applied. This is why we specifically separate between the notions of Transient Composite, Entity Composite, Value Composite and Service Composite. Because they're different, and it's important that they are different. Treating them differently, as opposed to "all is just the same", gives us advantages. You can easily parallel this with the problems with SOAP, where everything "is just a POST to a URL", which then totally removes all of the goodness of HTTP and URL's.

Similarly we have to come value having injection annotations that actually mean something. So instead of having something like a generic @Inject annotation, which can mean anything and therefore nothing, we have opted to use annotations which specify the scope of the target object. The main annotations we use are therefore @Service, @Uses and @This. @Service means that a service must be injected. @Uses means that an instance in an instance graph (e.g. the M in a MVC setup) will be injected. @This means that I want to get a reference to "myself" but cast to a specific interface, which makes sense in a Composite consisting of a bunch of mixins. Sometimes an interface injection for a specific type can be resolved both by @Service (="I want an external service") and @This(="I am a service and want to call this interface on myself"), so differentiating between them is essential.

All of these have different meanings, and all of them not only help the framework perform injections but also makes the code easier to read. Here's an example:

@Inject
void init(Foo foo, Bar bar, Some some)
{
...
}

Which of the above injections refer to services? You don't know? Well, of course not, because the information you need to know this isn't there. "It's so flexible, because Foo, Bar and Some can be anything!". And this is why you can't really do anything useful with them. If it's a service then Foo won't have any state, and you will be sharing it with lots of other instances. If Foo is an injected new instance then you'll own it and can treat it very differently from how you would treat a service. And so on. But since this information isn't there you can't really know what you can do with it.

By contrast, this is how it'd look like in Qi4j:

void init(@Service Foo foo, @Service Bar bar, @Uses Some some)
{
...
}

Now it is clear what scope each parameter is supposed to be injected from, which not only makes it easier to read the code (i.e. the annotations are documentation and not only framework instructions), but there's also some safety in it as there's no way Foo could accidentally be provided by an instance, and Some could not be provided by a Service. The parameters have meaning beyond their type: you KNOW what you can expect from them!

This is the main reason I think generic annotations like @Inject is a terrible idea. I wouldn't mind at all having a standard annotation that I can annotate my @Service, @Uses and @This annotations with though. That would make total sense, and would make it easier for IntelliJ to know that the field I just marked with @Service is going to be injected so don't show a warning if it's not initialized.

Then there are a number of details about the proposal that are also not quite thought through. As you can see the Qi4j annotations are applied on a per-parameter basis rather than on the method. This makes it easier for us to mark optionality for single injections rather than the whole thing. Here's how you'd have to do it with the suggested @Inject annotations:

 

@Inject(optional=true)
void init(Foo foo, Bar bar, Some some)
{
...
}

@Inject
void init(Foo foo, Bar bar)
{
...
}

By contrast, here's the equivalent Qi4j method:

 

void init(@Service Foo foo, @Service Bar bar, @Optional @Uses Some some)
{
...
}

There's only one method, and I can mark the @Uses injection as optional. If no match can be found it will simply be set to null. Same goes for constructors. The above even has use for me if I use this class without DI at all, since the annotations are not only injection instructions, they also form a sort of documentation for how to use it. The @Optional annotation can be used for regular method calls as well, and for properties. This makes it much much easier to implement and document null parameters and properties (sidenote: I think @Optional is MUCH better than the @Nullable suggestion in JSR-305).

This is also why the @Scope annotation is entirely backwards. It's set on the thing that implements the type rather than the injection point. This makes it an instruction to a framework rather than a declarative constraint on what can be injected. This in conjunction with the generic @Inject annotation allows you to do really weird things. An example:

 

interface Log {
void log(String message);
void setThreshold(int threshold);
}
@Singleton
class LogService implements Log {
void log(String message) { ... }
void setThreshold(int threshold) { ... }
}

class LogImpl implements Log {
void log(String message) { ... }
void setThreshold(int threshold) { ... }
}

@Inject
void init(Log log) {
log.setThreshold(3);
}

Can anyone tell me the result of setting the threshold? Is it just for the Log instance that I injected, or is it global? You don't know? That's because if "anything can be anything", there's no way for you to know! You are at the mercy of whoever wired this, and whoever wired it may not be the person who wrote the injection point, so they might make different assumptions of what injecting "Log" and setting a threshold on it means. Since there's nothing anywhere that says anything about it, anything goes. What a mess! If the @Scope had instead been on the injection point this would have been crystal clear, both for the class being injected, and the class providing the implementation.

@Named is equally problematic. You might think that it is simple: if there are many instances that implement an interface, just pick the one you want by specifying its name. This, however, is also backwards: the injection point has a strong dependency on the thing that is to be injected, and there's no way to change it without changing code. You also have problems if there are two injection points that declare the name of the injected thing, but they use two different names even though you, as the application assembler, want them to point to the same instance. What to do? Pray that your framework allows you to set two names for one instance seems to be the only way to solve it.

By contrast, in Qi4j an instance may indeed have a name set, but this should rarely if ever be referenced in code. Instead there's also a possibility to set tags (note the plural) on the service, which makes it easier to do declarative injections. One injection point might say "I need a Service Foo that has the tag xyz" and another injection point might say "I need a Service Foo that has the tag abc". With tags this is easily resolved as you simply add both xyz and abc as tags on the service. Problem solved! Fortunately this problem with the @Inject proposal can be trivially resolved: just rename @Named to @Tagged and you're good to go!

Provider is also weird, because it is trying to do two things at once: provide lazyloading of services and a factory for creating new ones. Example:

@Inject Provider<Foo> foo;

foo.get() can now be used to either lazyload an instance or create new instances of Foo. But let's say I am expecting it to be used for lazyloading of services. Then, if the framework during initialization finds that there is no service that implements Foo, I want it to abort. I.e. the above is close to a regular service injection, only with a lazyloading tweak to it. But, the framework has no way to know that this is what the developer intended, and so will happily start up and only upon foo.get() will you get an exception.

By contrast, in Qi4j the lazyloading service reference would look like this:

@Service ServiceReference<Foo> foo;

Now it is clear that if there are no Foo services available the application shouldn't even startup: it should throw an "non-optional service injection not resolved" type of exception. If I instead wanted to list all services implementing Foo, which might be zero, I would instead do:

@Service Iterable<Foo> foo;

This allows me to call foo.iterator() and walk through the service instances, which might be zero. If there are none available then the application will still start up normally. This more clearly shows the intent of the developer, and what is expected from the injection, rather than being a generic "gimme something that can gimme something", which, again, means nothing.

There is a common theme among the problems I have outlined: they are all stemming from the fact that the annotations are heavily influenced by the framework needs, rather than helping describe what your code is trying to do. They are imperative rather than declarative. "Do this" vs "This is what I need". This is more than just a technical detail: it is a fundamental principle. And from what I can see right now, it's all imperative.

To conclude, while Bob Lee asserted in the announcement that this would be a non-controversial spec, I would rather say, based on the above, that it's all backwards and should be rethought from scratch. There is, of course, a big problem with this: there are a LOT of investments in the technologies that are backing this proposal, and a rethink along the lines I have suggested is therefore unlikely to be feasible. I have respect for that, even though I obviously don't agree with the technical side of it. The best solution I can think of right now is to simply drop the spec. Why standardize on something that is known to be bad ideas, even if they are very commonly used? Then again, people tend to do what they want to do regardless of whether it's a good idea or not. Time will tell on this one.

From http://www.jroller.com/rickard

The Java Zone is brought to you in partnership with AppDynamics. AppDynamics helps you gain the fundamentals behind application performance, and implement best practices so you can proactively analyze and act on performance problems as they arise, and more specifically with your Java applications. Start a Free Trial.

Topics:

{{ parent.title || parent.header.title}}

{{ parent.tldr }}

{{ parent.urlSource.name }}