should a class with a single public constructor with parameters mandate the @Inject annotation?
James Strachan
still must add the @Inject annotation. I just wondered what the
rational was for this - it kinda feels a tad overzealous? I kinda like
the idea of encouraging folks to write classes with a single
constructor (similar to the google collections folks found when
writing collections) and as a small side benefit it would be kinda
nice that they can omit the @Inject.
I guess the worry is, if you refactor the code and add a new
constructor, code might break. But it'd be pretty obvious what broke
as you make the refactoring, so it'd be easy to add the @Inject when
you do want to add another constructor. Any other reasons for this
strict rule?
--
James
-------
http://macstrac.blogspot.com/
Open Source Integration
http://open.iona.com
Robbie Vanbrabant
Robbie
James Strachan
> Is that going to play nice with inheritance? Let's say you have a
> inheritance chain of three levels, and next to a public constructor (with
> args) in the parent type you add a new constructor. Now the two types below
> have an extra constructor, and Guice will blow up if you use one of them.
I didn't think guice invoked constructors in sub-classes? I thought it
just invoked the outer-most constructor? If thats true it doesn't make
any difference if the base classes constructors have @Inject
annotations or not.
> So
> now you have to know that it's actually the parent type that causes the
> other two types to fail (obviously you could also annotate the parent's
> constructor, but you don't always use the parent type with Guice).
If you derive from a class with 1 non-zero arg contructor; then later
on someone adds another constructor to the base class - you have to
explicitly expose that constructor yourself in the derived class
right? i.e. guice would keep on working fine in the derived class
unless you choose in the derived class to (i) expose another
constructor or (ii) switch the constructor being used
Robbie Vanbrabant
Robbie
James Strachan
> It seems if you have an object with a single public constructor - you
> still must add the @Inject annotation. I just wondered what the
> rational was for this - it kinda feels a tad overzealous? I kinda like
> the idea of encouraging folks to write classes with a single
> constructor (similar to the google collections folks found when
> writing collections) and as a small side benefit it would be kinda
> nice that they can omit the @Inject.
>
> I guess the worry is, if you refactor the code and add a new
> constructor, code might break. But it'd be pretty obvious what broke
> as you make the refactoring, so it'd be easy to add the @Inject when
> you do want to add another constructor. Any other reasons for this
> strict rule?
Bob/Jesse - any feedback on the reasoning for this restriction -
forcing a mandatory @Inject on a class with a single public
constructor with arguments?
limpb...@gmail.com
On Oct 14, 2:05 am, "James Strachan" <james.strac...@gmail.com> wrote:
> Bob/Jesse - any feedback on the reasoning for this restriction -
> forcing a mandatory @Inject on a class with a single public
> constructor with arguments?
code. If we supported lone constructors, we'd get
into trouble that way.
Gili Tzabari
What about no-arg constructors?
Gili
Bob Lee
What about no-arg constructors?
Ideally, we'd allow only default public no-arg constructors, because there's nothing to annotate and adding an explicit constructor is already equivalent to deleting the implicit no-arg constructor, but we may end up being a little more permissive to maintain compatibility w/ 1.0.
Bob
Gili Tzabari
I am beginning to think that maybe you should bite the bullet and
only require @Inject if multiple constructors are present. Yes, this
would mean that adding a new constructor might break existing code but:
1) It feels like a better default. One of the great things about JPA is
that is comes with great defaults out-of-the-box. It doesn't force you
to annotate fields when it can guess what you'd want it to do. If you
don't like the default you can always override it by adding an explicit
annotation. Why doesn't the same line of reasoning follow for Guice?
2) It will allow Guice to inject classes that cannot be modified, such
as 3rd-party libraries we do not have the source-code for. Having said
that, I'm not sure how practically useful this is.
On the flip side, if you require @Inject why not go the extra step
of requiring developers to provide an explicit constructor with a
@Inject annotation? The current behavior "breaks compatibility" when a
user introduces a custom constructor for the first time. In that sense,
it really isn't much better than moving from one custom constructor to
two...
Gili
Bob Lee
On the flip side, if you require @Inject why not go the extra step
of requiring developers to provide an explicit constructor with a
@Inject annotation? The current behavior "breaks compatibility" when a
user introduces a custom constructor for the first time.
The same is true for plain Java.
Bob
Gili Tzabari
I believe this ties into Joshua Bloch's "good APIs have good
defaults" paradigm (I remember him saying this at some point). In other
words, don't make the user do anything that you can reasonably do for
him automatically. I don't know whether it applies in this context but I
suspect it does. I don't think there is any danger that the lack of
@Inject will somehow reduce the readability of the code. It is pretty
clear that if there is only one constructor and Guice somehow constructs
an instance of that class than it must obviously be using that constructor.
I might buy that explicit @Inject has other benefits, but I don't
see the readability argument.
Gili
Bob Lee
I believe this ties into Joshua Bloch's "good APIs have good
defaults" paradigm (I remember him saying this at some point). In other
words, don't make the user do anything that you can reasonably do for
him automatically. I don't know whether it applies in this context but I
suspect it does. I don't think there is any danger that the lack of
@Inject will somehow reduce the readability of the code. It is pretty
clear that if there is only one constructor and Guice somehow constructs
an instance of that class than it must obviously be using that constructor.
Ah, but when you're looking for callers in a non-Guice-aware IDE (or just plain reading the code), the @Inject makes it obvious that this class is injected by Guice. That wouldn't be immediately obvious otherwise.
FWIW, I'm pretty sure Josh would agree with me here.
Bob
Gili Tzabari
I don't feel strongly about @Inject one way or another but the more
I hear in favor of explicit @Inject the less sense it makes to me.
1) Why would you want to identify these classes in the first place? I
mean, who cares whether the class is created by Guice or by someone else?
2) Is this ability worth the cost of explicit annotations? Is it worth
losing the ability to inject/integrate with 3rd-party libraries?
3) Doesn't @Inject just mean that it *can* be injected by Guice, not
that it necessarily is? Don't user Modules already give you most of this
information anyway?
Sorry to beat a dead horse :) I'm also surprised that no one else in
the community has an opinion (either way) on this matter...?
Gili
Bob Lee
1) Why would you want to identify these classes in the first place? I
mean, who cares whether the class is created by Guice or by someone else?
Tools, anyone who's reading the code and cares to debug where the arguments are coming from.
2) Is this ability worth the cost of explicit annotations? Is it worth
losing the ability to inject/integrate with 3rd-party libraries?
Yes. Provider methods will make 3rd party integration easy enough.
3) Doesn't @Inject just mean that it *can* be injected by Guice, not
that it necessarily is? Don't user Modules already give you most of this
information anyway?
Yes, it means that it can be injected, and no, modules don't give you all the information. Consider JIT (formerly "implicit") bindings.
Sorry to beat a dead horse :) I'm also surprised that no one else in
the community has an opinion (either way) on this matter...?
TMK, Jesse, Kevin, and I all agree.
Bob
Brian Pontarelli
> in
> the community has an opinion (either way) on this matter...?
7 characters, 3 keystrokes in most IDEs via a template, no compile or
runtime requirements, extra information for reflection, I don't see
the problem really. I mean it's just "gi<tab>" and your done
(gi="Guice Inject" for those wondering,
emsm="EasyMock.createStrictMock" in my world). :)
-bp
Gili Tzabari
I am more worried about integration with 3rd-party
libraries/frameworks than anything else. I agree that @Inject is easy to
add, but that requires you to have control over the source-code which is
not always the case. I will feel much better once I see what is rolled
out to address this use-case. I broke my teeth a bit trying to integrate
Guice into JAX-RS :) There were a bunch of classes that Guice could
easily inject, but it is technically impossible for me to annotate them
with @Inject because I don't control the code and that same code also
has to work with Spring and other IoC frameworks.
Gili
Brian Pontarelli
Just write some CGLIB to sub-class everything that has a single
constructor without the @Inject, override the constructor and add the
annotation. What like maybe 10 lines of code or so? ;)
-bp
James Strachan
> On Tue, Oct 14, 2008 at 5:25 PM, Gili Tzabari <gili.t...@gmail.com>
> wrote:
>>
>> I believe this ties into Joshua Bloch's "good APIs have good
>> defaults" paradigm (I remember him saying this at some point). In other
>> words, don't make the user do anything that you can reasonably do for
>> him automatically. I don't know whether it applies in this context but I
>> suspect it does. I don't think there is any danger that the lack of
>> @Inject will somehow reduce the readability of the code. It is pretty
>> clear that if there is only one constructor and Guice somehow constructs
>> an instance of that class than it must obviously be using that
>> constructor.
>
> Ah, but when you're looking for callers in a non-Guice-aware IDE (or just
> plain reading the code), the @Inject makes it obvious that this class is
> injected by Guice.
Not really - it indicates it *may* be injected by Guice but until
you've looked at uses of the Class you don't know how its being bound.
e.g. someone may be invoking the constructor directly in a provider
method or binding an instance directly.
If a class has only one constructor and its public and it has
arguments - you *know* its being invoked - from anywhere (guice or
regular java code) - whether it had an @Inject or not - so this seems
a silly argument to me :)
So there's definitely plusses and minuses to this.
Sure adding a new constructor breaks things - but then it would break
things very fast (in the same way that adding a parameter to a
constructor might break things too). Folks should run unit test cases
so catch these breakages pretty quickly.
On the plus side - think of the zillions of Java classes out there
that Guice would be able to automatically inject without folks having
to write their own magic AOP bytecode swizzler, get access to the
code, hack it and re-release it or write custom providers.
--
James
-------
http://macstrac.blogspot.com/
Open Source Integration
http://fusesource.com/
Dhanji R. Prasanna
So there's definitely plusses and minuses to this.
Sure adding a new constructor breaks things - but then it would break
things very fast (in the same way that adding a parameter to a
constructor might break things too). Folks should run unit test cases
so catch these breakages pretty quickly.
On the plus side - think of the zillions of Java classes out there
that Guice would be able to automatically inject without folks having
to write their own magic AOP bytecode swizzler, get access to the
code, hack it and re-release it or write custom providers.
James Strachan
> On Wed, Oct 15, 2008 at 5:05 PM, James Strachan <james.s...@gmail.com>
> wrote:
>>
>>
>> So there's definitely plusses and minuses to this.
>>
>> Sure adding a new constructor breaks things - but then it would break
>> things very fast (in the same way that adding a parameter to a
>> constructor might break things too). Folks should run unit test cases
>> so catch these breakages pretty quickly.
>
> Yea but the point is that break is caught at compile time in idiomatic Java,
> and doesn't require unit testing.
I don't follow this one - have I missed something? Things would only
fail if you had a unit test attempting to bind the class (without a
provider method) when creating a Guice injector right? Or is there
something else I'm missing?
The argument seems to be between 'mandatory' or 'recommended'.
e.g. if Guice *recommended* you add an @Inject to single constructors
with arguments as good practice (it could even generate a warning if
it injected classes without an @Inject annotation) - then folks can
choose to follow the recommendation and still have their code break if
someone adds another constructor - or folks could still bind any old
Java class they find from the zillions of Java libraries out there if
they have a single constructor without having to write a provider
method for every class/constructor combination.
>> On the plus side - think of the zillions of Java classes out there
>> that Guice would be able to automatically inject without folks having
>> to write their own magic AOP bytecode swizzler, get access to the
>> code, hack it and re-release it or write custom providers.
>
> I can see the merit in this argument.
Given that not enforcing @Inject gives lots of benefits to folks using
external code and folks who want to can still stick to adding an
@Inject everywhere just in case - you can see which side of the fence
I sit :). My gut feel is that forcing the addition of an extra
mandatory annotation on a perfectly well written Java class before
Guice will even consider injecting it seems kinda anti-Java; when
folks can use @Inject in the way you'd like (to make things break if
folks refactor).
It'd be pretty easy to use a compile time tool (e.g. a little
annotation processor) to mandate any project's code always uses an
@Inject on a constructor within a certain package for those folks who
wanna lock down codebases.
There's a massive difference between owning an entire codebase and
being able to enforce coding styles to aid refactoring - and being
easily able to reuse an external library. Maybe make this a flag then?
Strict mode is on by default but it can be disabled if folks wanna use
code they don't control & wanna avoid writing loads of unnecessary
provider methods?
Brian Pontarelli
> that Guice would be able to automatically inject without folks having
> to write their own magic AOP bytecode swizzler, get access to the
> code, hack it and re-release it or write custom providers.
But every system needs at least one swizzler. Otherwise, how will you
guarantee maintenance for the next 20 years? ;)
I'm coming around to this idea though. Things currently blow up at
runtime if you haven't written a provider and there isn't a no-arg
constructor. Is blowing chunks for multiple constructors any different?
hmmmm......
-bp
Bob Lee
There's a massive difference between owning an entire codebase and
being able to enforce coding styles to aid refactoring - and being
easily able to reuse an external library. Maybe make this a flag then?
Strict mode is on by default but it can be disabled if folks wanna use
code they don't control & wanna avoid writing loads of unnecessary
provider methods?
There's also the possibility of requiring @Inject for JIT bindings but not explicit bindings with public constructors. Explicitly binding a class is almost as good as applying @Inject.
Bob
Gili Tzabari
JAX-RS takes the strategy of choosing the constructor with the
greatest number of injectable objects. Guice could do the same by
default and @Inject would be used to override it.
Gili
Sam Berlin
choose an injectable constructor, even if the pattern is defined
somewhere. I am a very strong fan of Guice's explicit @Inject
behavior when multiple constructors are involved.
As far as a single constructor goes... I'm ambivalent about it. I've
written up about 6 or so emails to respond to this thread and
discarded them all halfway through writing, because I don't really
care either way.
But multiple constructors... @Inject is definitely necessary there.
Sam
Bob Lee
We can't take this decision lightly. While we can safely relax the rules at any time, we can never make them more strict, so we have to make sure this is the right decision or else live with the consequences.
Bob