Something slightly weaker than requireAtInjectOnConstructors

[Tavian Barnes]

I'd like to enforce a restriction like Guice 4's requireAtInjectOnConstructors() but slightly weaker: any class with less-than-public effective visibility may have a non-annotated, no-arguments constructor.  This is because I have a lot of package-private implementation classes, and it's a pity to have to change

class ThingImpl implements Thing {

    ...

}

into

class ThingImpl implements Thing {

    @Inject

    ThingImpl() {

    }

    ...

}

for all of them, with no real benefit.  On the other hand, it is helpful to be notified of public classes which accidentally have a public, non-annotated constructor.

It seems this is almost possible with TypeListener, except it will also trigger on requestInjection()/injectMembers().  Any ideas on how I could do this?

[Sam Berlin]

I don't know if there's any way to do exactly what you'd like.  requireAtInjectOnConstructors works the way it does because it's intended to prevent accidentally creating things that aren't meant to be injected (but do have no-args cxtors).  Accidentally constructing these kinds of things can happen whether or not the implementation is public, package-private or private... so that's the logic behind the check right now.

We generally use requireAtInjectOnConstructors internally, and folks just sprinkle no-args constructors with @Inject where necessary.  Note that if you use bind(..).toConstructor(cxtor), that doesn't require the @Inject -- so if you have some kind of framework creating bindings for you, you could potentially use that.

 sam

--
You received this message because you are subscribed to the Google Groups "google-guice" group.
To unsubscribe from this group and stop receiving emails from it, send an email to google-guice...@googlegroups.com.
To post to this group, send email to google...@googlegroups.com.
Visit this group at http://groups.google.com/group/google-guice.
For more options, visit https://groups.google.com/groups/opt_out.

[Christian Gruber]

"to no real benefit"

I think the benefit is clarity, and avoiding a class of error. Given
that you do want to differentiate between certain classes of constructor
on the basis of visibility, you do want this benefit, it's just that you
want it on the basis of a convention, rather than an annotation. That's
cool, but I would humbly suggest that the annotation was created for
just such a purpose - to indicate intent, and the restriction was
designed to enforce the intent - that this is an "injectable" class.

So... I guess it's an opinion, but I certainly don't feel that it's "to
no real benefit" even if that benefit is "merely" executable
documentation of intent and code-clarity.

Just my 10,000 satoshis,
Christian.

Christian Gruber :: Google, Inc. :: Java Core Libraries :: Dependency
Injection
email: cgr...@google.com :::: mobile: +1 (646) 807-9839

[Tavian Barnes]

Okay good points, I guess I'll just bite the bullet and add them. It's not too bad anyway, since I'd end up adding constructors to those classes if they ever gained a dependency anyway. No reason to special-case package-private classes with no dependencies just to save three lines of code each.

[Geoff Groos]

I'd like to bump this but with package names rather than visibility restrictions.

I too have data models that I don't want to be constructed by giuce, since they should almost always be used with @Assisted factory methods bound toInstance, but I would really like it if utility classes, (in particular and in my case, javafx.stage.DirectoryChooser, a class with an unannotated no-arg ctor) could be wired up automatically.

We use requreAtInjectOnConstructors universally since it completely avoids a whole class of bugs around bad-binding of data objects. Right now I'm debating making a change toward more IOC with a utility class that wraps the Directory chooser (along with a couple other things). I would really like the issue to be as simple as "inject the DirectoryChooser, add tests to cover currently untested code, devs are happy", but it isn't that simple because I'd have to go off to a number of modules and update their configuration to bind(DirectoryChooser.class).toProvider(DirectoryChooser::new). I don't want to make those changes.

An elegant solution to our problem would be to change the requireAtInject to support a package prefix: requireAtInjectonConstructors("com.mycompany"), which would require that everything in a package starting with "com.mycompany" uses the annotations, and everything in other packages (eg javafx.whatever) does not.

Of course, this would change guice's own model from using a single boolean flag to using a set of filters, which is obviously substantially more complex.

My .02, thanks for reading

-Geoff

[Tavian Barnes]

You know, since I made this thread I've changed my mind, I think requireAtInjectOnConstructors() is perfect the way it is, and Guice probably shouldn't add any more complexity around it.

But I agree that sometimes it's nice to have a weaker requirement.  For example, when shipping a library, I'd like to make sure that all my library's injected classes have @Inject on their constructors, but not require that for consumers of my library.  This is a case where requiring @Inject annotations only in a particular package (and maybe its subpackages) would be nice.

Luckily you can do this yourself, either with a TypeListener or Elements.getElements().