Rickard Oberg is popular among Java developers. He has given seminars at all main Java conferences world wide. He worked as an architect at JBoss and other popular OpenSource Java frameworks, and wrote a book on RMI. In recent years, he has become famous as an Aspect Oriented Programming (AOP) crusader. He has worked with bleeding edge AOP in a portal product that has become a great commercial success, and is currently working on Qi4j at Jayway. Rickard is a DZone MVB and is not an employee of DZone and has posted 16 posts at DZone. You can read more from them at their website. View Full User Profile

Why @Inject is a Bad Idea

05.11.2009
| 6891 views |
  • submit to reddit

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

Published at DZone with permission of Rickard Oberg, author and DZone MVB.

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Tags:

Comments

Jakob Jenkov replied on Mon, 2009/05/11 - 2:56am

I couldn't agree more, that @Inject is a bad idea. But not because it is too loose in specifying what to inject. Rather, I think the whole idea of using annotations to mark what to inject is inappropriate. Annotations just don't cover enough use cases.

For instance, how will a DI container differentiate between these two constructors?

 

public class MyClass {

  protected Dependency dependency = null;

  public MyClass(){
    this.dependency = new DefaultDependencyImpl();
  }

  @Inject
  public MyClass(Dependency dependency){
    this.dependency = dependency;
  }

}

 

I mean, should the container call the no-arg or the 1-arg constructor? You will have to specify that
in the place where the MyClass is to be injected. @Inject is not by itself enough to tell that.

 

Second, you have problems with third party code that does not have these annotations in them.
If you don't have the code for some component you are using, you cannot insert any @Inject into
that code. Hence, you have to use some other mechanism to wire up that component. That is also
true for the Qi4J annotations.

All in all, I just don't think annotations is the right mechanism for wiring up components, since there
are lots of little cases they cannot cover.

Personally, I think a DSL for dependency injection can be made much more flexible, and cover a lot
more use cases. That is why I implemented a DSL for Butterfly DI Container. This language covers
a lot more use cases than annotations can, and it makes it very easy to switch into pure Java code
when you need to, during object wiring / creation. For instance, how do you use annotations for
application configuration? Or for internationalization? Or how do you call methods in other classes
than the one being instantiated, like registering the newly created instance with a static instance counter? Butterfly Container Script can do all that, and more.

 

Michael Stachel replied on Mon, 2009/05/11 - 4:34am in response to: Jakob Jenkov

Hi,

First: your example has a fundamental design flaw in it. You define an implementation dependency (DefaultDependencyImpl) to your class (MyClass) although you have an Interface (Dependency) designed. The use case that you want to fallback to a default implementation if no other implementation is defined is a kind of strategy which the DI container either Spring or Guice can easily handle for you.

Second: you cannot inject a third party class, if you have no source and want to change it. This is true for all DI containers. Therefore this is IMHO the most important aim of this JSR. To provide class protability!

The rest is noisy advertising :-)

 

Michael Lee replied on Mon, 2009/05/11 - 5:43am

I disagree.  Doing things in abstraction makes them much easier to change and understand.  What would happen if you wanted to switch the @Service to something else?  Having so many options to describe how something gets injected is great for someone intimately familiar with the code base, but serves as a detriment to someone unfamiliar with an unknown amount of annotations.  Your examples are fair in context but for someone consuming the code base they shouldn’t have to care if something is a service or a reference to itself as long as it’s initialized and works.  

Also, I love annotations and what they offer but there needs to be some sort of standard on the topic.  I see my team and myself writing more and more annotations with each project. 

Piero Sartini replied on Mon, 2009/05/11 - 7:49am

For me, @Inject is low-level infrastructure for the code, in contrast to the qi4j annotations which help with my domain architecture. For simple usecases they could be overkill.

Let's take a look at tapestry for example.. there is tapestry-ioc which defines the core di annotations and on top of them are the more specific annotations like @InjectPage, @Component and others that make sense in the context of a webapplication.

Couldn't @Inject be seen as the low-level annotation which gets refined by @Service, @This and others?

Jordan Zimmerman replied on Mon, 2009/05/11 - 1:43pm

This article proves why @Inject should not be part of the formal Java spec, but not in the way it intends. IoC is a young (and for me unproven) concept. Let's let individual teams choose whatever method of IoC they want (preferrably none at all). There is no good reason to add this to the language. Doing so will forever lock Java into a single IoC approach.

Jakob Jenkov replied on Mon, 2009/05/11 - 3:27pm

@Tirex,

Design Flaw? Why? 

Think of MyClass as some component I want to share with the world. An API, in other words. If I want someone else to be able to use it with / without default implementation, do I have any other option than the one I showed above? I don't really think so (but I could be wrong...). The no-arg constructor is a convenience method. How can I know if the user uses a DI container or not? Hence, I need both options. There is absolutely no design flaw. It works as intended.

 

Noisy advertising? Well, how can I argue against annotations, and for something else, without providing a reference to what I think is the better option? Try that in university, and you'd fail your assignment. And, by the way, isn't Rickard Øbergs mentioning of Qi4J in an article titled "why @Inject is a bad idea" noisy advertising too, then?

 

Like I said... annotations are in my opinion a 50% solution. I go for 100% or nothing (well, 99% will probably satisfy me too). I've also considered annotations for a persistence API I've done (no advertising this time...!). But you know what? There were still many use cases annotations couldn't cover. I'm sorry, but annotations just isn't my cup of tea except for class static configuration, and I'll keep pointing out the holes in annotations until I die (or get tired of it).

Bob Lee replied on Mon, 2009/05/11 - 4:08pm

Ugh. Why is the exact same content in two different places, each with its own set of comments? I've had a comment pending moderator approval on Rickard's blog since yesterday, so I'll go ahead and summarize my response here.

1) Rickard prefers to use the domain modeling-specific term "service" instead of "singleton". DI is useful for much more than just domain modeling, the term "service" is terribly overloaded already, and more people will know what "singleton" means. @Singleton it is.

2) It sounds to me like Rickard is the one who has "not quite thought through" optional injection. I prefer to minimize the surface area of my APIs, and I don't think optional dependencies are useful enough to justify a top-level type. Allowing a single parameter to be optional means you have to pass in null and you can't support primitive types (you must use wrapper types). I prefer to not inject the member at all.

3) I'm not sure why you would want to specify the scope at every injection point. If I say "@Singleton Foo" at an injection point, does that mean I want the singleton version of Foo or that I want the same instance of Foo to be passed to that injection point every time? Specifying the scope in one place works great. If I really want to specify the scope on a per-injection point basis, I can just use qualifiers. We don't have the use cases to justify forcing every injector to implement this feature. Nothing in the spec precludes Qi4j from supporting it though.

4) @Named is useful for quick and dirty integration tasks. I agree that using qualifiers is more maintainable in the long run, but those typically require more explicit configuration, so it's a tradeoff.

5) Guice fails at initialization time or even build time if a Provider<T> can't be resolved, not run time.

6) "[The annotations] are imperative rather than declarative." @Inject has an imperative name, but it and the qualfiier and scope annotations are very much declarative.

7) "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."

Your opinion is duly noted.

"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."

This spec will not be constrained by existing implementations. Standards are forever. It would be silly to weigh it down with today's implementation concerns.

"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."

I'm sorry, but you're a long way from proving that Qi4j is a good idea, let alone that @Inject is such a bad idea that we should drop it entirely. As much as I'd love to have mixins, Qi4j is too magical for my taste.

Bob

Simon Martinelli replied on Tue, 2009/05/12 - 6:13am

In my opinion @Inject goes into the right direction and I like to thank Bob and Rod for the effort to provide the spec as a JSR!

A while ago I created a small DI framework called simject (yes this is self advertising ;-) for self education on DI which I will adapt to the new spec for sure.

Currently simject only uses the @Resource annotation which already is in Java 6 API to tell the framework that there must be something injected. What has to be injected is definied in a small XML configuration file. This will avoid some magic behavior on runtime.

I don't want to compare simject with Spring or Guice but sometimes a lean approach is better than a overengineered, overloaded framework that can support every case even if this case will never happen.

So let's go with the @Inject spec and keep it simple or small but beautiful ;-)

Michael Stachel replied on Tue, 2009/05/12 - 6:26am in response to: Jakob Jenkov

@Jabob Jenkov

Design Flaw? Therfore!

Flaw:  Violating of DIP (Dependency inversion principle). Your API class MySystem has a dependency on a low level class (DefaultDependencyImpl). In case of changes there you have to touch your API class as well.

A further drawback is that you prevent an injection into the DefaultDependencyImpl class because you instantiate it itself in the API, unless your system consists only of these 2 classes which would then hardly demand an DI container at all.

I am with you that annotations aren't a 100% solution. In contrast I agree wholehartedly but I find this is exactly the point of this JSR approach. It does only define a standard on the lowest common denominator and @Inject is mostly all you need. In contrast look at JSR299 or even at JPA. They try to invent a whole DSL on annotations which I completely find is an abuse of annotations. 

 

 

 

Jakob Jenkov replied on Tue, 2009/05/12 - 7:49am

@Tirex

 

>Flaw:  Violating of DIP (Dependency inversion principle). Your API class MySystem has a dependency on a low
>level class (DefaultDependencyImpl). In case of changes there you have to touch your API class as well.

:-) ... If you look at the code again, you will see 2 constructors. One which uses a default implementation of Dependency, and one which takes a Dependency as parameter. If you are only going to use the default implementation anyways, why even inject it? Use the no-arg constructor. If you need to use a non-default Dependency, use the second constructor. Of course this only works if the default implementation needs no arguments, or is instantiated with default arguments. But like I said, that no-arg constructor is a convenience method. You will see that advice repeated in API design guides, like for instance in Joshua Block's "Effective Java":

"Provide sensible defaults".

 If you give people an API in which they have to assemble every component from scratch, even when they are just using default implementations with default arguments, you will get some annoyed users.

 

So, you call it "design flaw", I call it "convenience constructor". 

 

This little discussion emphasizes another problem in software design: Personal preferences. I have worked with lots of developers who think somebody elses code is crap. And then their own code isn't much better, just different. I have also read so many discussions in which two developers discuss for and against some way of doing things, and in the end, it doesn't really matter which of the two methods are used. The code works. Too much is personal preference. Therefore I try not to call somebody elses code a "design flaw", unless that code doesn't do what it was designed to.

Mladen Girazovski replied on Tue, 2009/05/12 - 8:38am

Jakob Jenkov,

i agree on the judgements about other peoples code (Fowler says to always assume that the author had lots of complex problems to  solve), but i'm not sure if Bloch's statement is appropriate here.

After all, the DI framework is supposed to take care of the object construction/assembly, so maybe i'm missing something when i say that there is no real point in providing a convinience constructor.

Jakob Jenkov replied on Tue, 2009/05/12 - 8:42pm

@Mgira

 

1) The DI framework is supposed to handle injection ONLY IF the user of MyClass is actually using a DI framework. But what if MyClass was part of an open source component I have developed, and I don't know if my users are using a DI framework or not? 

 2) There is still no reason to bother users with having to configure injection of a default implementation, is there? After all, the net effect is the same. Oh, except MyClass has an extra dependency on the DefaultDependencyImpl, but that's not dangerous here.

 

In my opinion, Bloch's comment applies to API design, regardless of whether the user of the API is using a DI framework or not. 

Michael Stachel replied on Wed, 2009/05/13 - 2:12am in response to: Jakob Jenkov

@Jakob Jenkov

The funny thing about your answer is that you are speaking about different (valid?) viewpoints of writing code but are heavily attacking @Inject programming style. Very special opinion at least!

Nevertheless the dependency from MySystem to DefaultDependencyImpl is an objective design flaw by violating common design principles. Even if you don't want to use a DI framework you could have removed this dependency with a factory.

My last 2 cents on this.

Bruno Sofiato replied on Thu, 2009/05/14 - 12:23am

This whole dependency injection by annotation think is as a whole a design flaw IMO. I may sound a little harsh, but, a class should not be aware of how and WHAT dependencies are supplied, in fact a class should not be aware that a DI framework would be used AT ALL.

jiji530 (not verified) replied on Tue, 2009/06/30 - 12:18am

thanks for your post.perhaps you will like abercrombie ed hardy mortgage rates tiffanys ed hardy Is not it?

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.