google / guice

Guice (pronounced 'juice') is a lightweight dependency injection framework for Java 11 and above, brought to you by Google.
https://github.com/google/guice
Apache License 2.0
12.42k stars 1.66k forks source link

Allow bindings in the same Module to be overridable #80

Closed gissuebot closed 10 years ago

gissuebot commented 10 years ago

From ajoo.email on March 19, 2007 16:28:04

It is a common use that a common Module is created to provided some default binding, and a few sub-modules that override some specific bindings.

When a Binder sees a binding being re-bound, it should be able to check the originating Module and decide not to blow up if they are the same Module.

Original issue: http://code.google.com/p/google-guice/issues/detail?id=80

gissuebot commented 10 years ago

From crazyboblee on March 19, 2007 13:54:20

What if we had an optionallyBind() method? The optional binding would not take if an explicit bind() exists elsewhere. If multiple optionallyBind() calls exist for the same key, you get an error. You can already do this to some extent using @ImplementedBy and overridding with explicit bindings.

gissuebot commented 10 years ago

From sberlin on September 18, 2007 16:50:06

The patch is complete for normal binding.  If folks approve of the way this is written, I'll finish it to include constant binding, static binding, & scopes.  (And clean up the formatting).

The only part I'm unhappy with is the proliferation of interfaces (DelegateModule, OverridingModule).  Unfortunately I couldn't think of any other way to make it work.  Ideally there would be a subclass of BinderImpl inserted, but because the binder is constructed in Guice (the class, not the framework), that wasn't really possible.  Suggestions on this front are very welcome.

(I'm also unsure what the deal is with newly added files being marked as executable.  They're not executable on my local machine.)

(I am attempting to attach a txt patch file, but keep encountering a server error...)

gissuebot commented 10 years ago

From sberlin on September 18, 2007 16:51:34

+ patch file

Attachment: gist    changes.txt

gissuebot commented 10 years ago

From viegas.dev on October 04, 2007 11:01:53

After reading your proposal I find it very usefull for when we want to overrida an intire Module. Wouldn't it be easier if we could overide one binding instead of the full module?

A simple optionallyBind() like crazyboblee proposed or a... binder.bind(Foo.class).to(Bar.class).overridable();   (on the default module) binder.bind(Foo.class).to(NewBar.class);              (on the new module)

...be a cleaner solution? The overriding programmer would not need to now the Module where the service was declared previouslly. More so he would not need to override several methods repeating all the contained services if he only wanted to override a few of them in each module.

I'm building a framework on top of Guice and plan to have several tens of services in a few Modules, so this is a very real issue for me if I want to allow programmers to override some default behaviour of the framework, without knowing it completly.

I started using Guice a few days so did not look at the code yet (besides your patch) so sorry but I can't gave a propor proposal of how implementing this.

gissuebot commented 10 years ago

From sberlin on October 04, 2007 12:41:25

The syntax in the patch actually does allow you to do this, it just might not be very clear from the wording of Modules.override(a).with(b). The proof-of-concept allows something like:  ModuleOne {     configure() {         bind(A.class).to(AImpl.class);         bind(B.class).to(BImpl.class);     }  }

 ModuleTwo {     configure() {        bind(A.class).to(AOtherImpl.class);     }  }

 Module over = Modules.override(new ModuleOne()).with(ModuleTwo());

The result is A is bound to AOtherImpl and B is still bound to BImpl.

gissuebot commented 10 years ago

From viegas.dev on October 08, 2007 02:23:08

Ah... I see... sorry, I got the impression that we had to override the whole module, with all it's services.

No more arguments from me... thumbs up on your proposal. Can't wait to get it on a new version! ;-)

gissuebot commented 10 years ago

From limpbizkit on May 17, 2008 20:56:16

An early version of this feature was written by Sam Berlin (and myself) and checked in. https://code.google.com/p/google-guice/source/detail?r=486 http://publicobject.com/2008/05/overriding-bindings-in-guice.html We need to decide whether to keep the API as-is or use a mini DSL. With the DSL, user code looks like this:   Module replaced = Modules.override(new ProductionModule()).with(new TestModule()));         ...vs the single method code:   Module replaced = Guice.overrideModule(new ProductionModule(), new TestModule()));

- the DSL allows varargs for original and replacement modules

Status: Started
Labels: -Type-Defect Type-Enhancement

gissuebot commented 10 years ago

From limpbizkit on May 30, 2008 01:11:05

Kevin, can you code-review and API review this new method? I think the implementation is solid but I'm not that happy with the method name and location.

Owner: kevinb9n
Labels: Milestone-Release2.0

gissuebot commented 10 years ago

From limpbizkit on August 02, 2008 14:28:48

I've replaced Guice.overrideModules with the new mini EDSL:   Module functionalTestModule = Modules.override(productionModule).with(testModule) https://code.google.com/p/google-guice/source/detail?r=588 Much thanks to sberlin for doing the work!

Status: Fixed
Owner: limpbizkit