frjaeger220 / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 0 forks source link

Allow bindings in the same Module to be overridable #80

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
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 reported on code.google.com by ajoo.em...@gmail.com on 19 Mar 2007 at 8:28

GoogleCodeExporter commented 9 years ago
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.

Original comment by crazybob...@gmail.com on 19 Mar 2007 at 8:54

GoogleCodeExporter commented 9 years ago
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...)

Original comment by sberlin on 18 Sep 2007 at 11:50

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
+ patch file

Original comment by sberlin on 18 Sep 2007 at 11:51

Attachments:

GoogleCodeExporter commented 9 years ago
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.

Original comment by viegas....@gmail.com on 4 Oct 2007 at 6:01

GoogleCodeExporter commented 9 years ago
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.

Original comment by sberlin on 4 Oct 2007 at 7:41

GoogleCodeExporter commented 9 years ago
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! ;-)

Original comment by viegas....@gmail.com on 8 Oct 2007 at 9:23

GoogleCodeExporter commented 9 years ago
An early version of this feature was written by Sam Berlin (and myself) and 
checked in.
http://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
- the DSL reads nicer, it's harder to get the modules in the wrong order
- the DSL requires an extra interface
- the single method is more traditional

Original comment by limpbizkit on 18 May 2008 at 3:56

GoogleCodeExporter commented 9 years ago
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.

Original comment by limpbizkit on 30 May 2008 at 8:11

GoogleCodeExporter commented 9 years ago
I've replaced Guice.overrideModules with the new mini EDSL:
  Module functionalTestModule = Modules.override(productionModule).with(testModule)

http://code.google.com/p/google-guice/source/detail?r=588

Much thanks to sberlin for doing the work!

Original comment by limpbizkit on 2 Aug 2008 at 9:28