17

I've heard claims that "@ImplementedBy is evil", on grounds that it breaks DI concepts and makes the interface aware of its implementors.

This might be true in some cases, but often I found that it just leads to cleaner code (no long Modules to maintain), while not really hurting anything in the process.

As pragmatics, not purists, when do you think it's worthwhile to use @ImplementedBy?

ripper234
  • 222,824
  • 274
  • 634
  • 905

5 Answers5

14

I had the same ugh, ick, yuck feeling of @ImplementedBy BUT at the same time, it's very useful. Spring has to scan all classes in the package list you give it. In Guice you don't have to configure that package list to scan and @ImplementedBy is key for that(if not using the Binder to bind that is). AS it goes down your object heirarchy on the first Injector.getInstance, and hits an interface, it then uses the @ImplementedBy to find the default implementation(as long as there is nothing in the Binder overriding that default).

We use @ImplementedBy as well. We find it extremely nice to use, it is a bit ugh, but it just works and works nicely and since it's DI, it is not really depending on the implementation since you can override bindings with new ones anyways.

At the same time interfaces are generally used less and less with DI frameworks. All the DAO interfaces went away on our project AND we can still swap in mock objects for the DAO. The java classes are implicit interfaces to begin with that can be mocked without needing the interface. We now reserve interface use for main apis to be very clear and not clutter it with implementation code. For DAO's we have not needed this anymore.

Matt Ball
  • 354,903
  • 100
  • 647
  • 710
Dean Hiller
  • 19,235
  • 25
  • 129
  • 212
  • Going through the classpath is a one-time startup cost. What I don't like about `@ImplementedBy` is that it creates a circular dependency. I'd have liked an `@ImplementorFor` instead. – Mihai Danila Jan 31 '19 at 00:21
  • true but ImplementedFor would be pretty expensive on startup as you would have to 'find' the ImplementedFor classes meaning scan EVERY class. Hibernate has this issue and then they put a nasty workaround to only scan jars with a meta.xml file in it as it was expensive....non-ideal scanning ALL jars. I agree with the theory but practicality wins in my view. @MihaiDanila – Dean Hiller Feb 13 '19 at 23:28
  • But is it impractical to scan the whole classpath once, on startup? I think it depends on the application, and startup costs are generally large for a system. Certainly something that should be decided by the user of the library, with the library offering us the option to use the feature. – Mihai Danila Feb 21 '19 at 13:42
  • yes, I guess if you are not doing something like this, it is fine https://docs.google.com/document/d/1XokDDBL4oPhvHvqcC4uNMGPHoVSeEWDvGz2crOLKXbQ/edit?pli=1 We do that for every server so for any server we do it is impractical making test suites take forever. Our startup time of our servers is quick and our test suites keep it that way(and we can refactor like no-ones business without needing to rewrite tests). so you are correct. I come from a different world. – Dean Hiller Feb 21 '19 at 21:44
  • @MihaiDanila ^^ – Dean Hiller Feb 21 '19 at 21:45
  • Integration tests, sure, they are allowed to be slow (and there should be very few of them). My unit tests use fakes or mocks at the unit boundaries, so Guice isn't really needed. Even if it were needed, it can be used in a controlled fashion without triggering classpath scans. So in my world, that startup cost does not get paid on unit tests. – Mihai Danila Feb 24 '19 at 23:17
10

You should generally prefer explicit bindings over just-in-time (JIT) bindings. Explicit bindings allow the injector to crawl the dependency graph at injector-creation time. This allows Guice to fail-fast if a dependency is missing or invalid. With just-in-time bindings like @ImplementedBy, Guice can't report the problem until the binding is exercised.

JIT bindings also interact poorly with PrivateModules/child injectors. Though most apps shouldn't need these features, when you do it's less painful if every binding belongs to a specific module.

Jesse Wilson
  • 39,078
  • 8
  • 121
  • 128
5

It is useful when an interface is not intended to have multiple implementations but has to be part of the dependency injection process because it has dependencies which have to be injected by the framework.

thSoft
  • 21,755
  • 5
  • 88
  • 103
3

I can see why Google did what they did, but having a preferred implementation of something isn't necessarily evil. Note the documentation says it is for the default implementation, not the only one.

Btw, I found this question because I was searching the Internet for existing implementations of the concept of @ImplementedBy.

I created an annotation called @ImplementedBy to place on one of my interfaces. When using pure, un-injected reflection, this is the simplest way to tell an interface what implementation to use, especially when working with existing APIs that only understand interfaces. (interface, not implementation)

The annotation allows me to genericize some really gnarly generators with one line of annotation and a one-line inside the decorator. I don't have to use a dependency framework for such a simple operation.

Elijah Sarver
  • 587
  • 6
  • 8
0

for me, yes it's evil if you use it to hard-wire-bind and never ever reuses the binding because you inverse the sense of an interface. I agree to @thSoft that this is a well common pattern but its even not clear to me, why we don't have a @Implements annotation. Also it might be irritating that the given default implementation is not the one that is used in runtime.

Just to be clear what google sayed to that

Annotate types tell the injector what their default implementation type is. …

!! It also dismisses generic interfaces like

@ImplementedBy(MyImpl.class)
public interface MyInterface<SIn,SOut> {}

public class MyImpl implements MyInterface<String, Integer> {}

that typically are not implemented only once. see Inject Generic Implementation using Guice for details.

Community
  • 1
  • 1
childno͡.de
  • 4,679
  • 4
  • 31
  • 57
  • The reason why there's no `@Implements` is that it makes no sense: 1. There may be multiple classes annotated with `@Implements(MyInterface.class)` and then you're lost. 2. Even worse, in order to find the implementation, you'd have to load all classes first. +++ Concerning generics, I guess, `@ImplementedBy` is just a shortcut for a simple usage and so it's good enough. AFAIK, there's no way to use a `TypeToken` as an annotation argument. – maaartinus Nov 15 '17 at 04:10
  • There may be multiple implementors, but it's still useful when used correctly. One could name it `@DefaultImplementor` to be more decisive about whether it can be used. `@ImplementedBy` creates circular dependencies when there's build systems behind the scenes. – Mihai Danila Jan 31 '19 at 00:24