Assisted Injection for Dagger

[Jesse Wilson]

There's been a lot of interest in doing assisted injection for Dagger. Let's all agree on the problem, propose some solutions, and decide whether that solution is worth its cost.

THE PROBLEM

I have a class that gets some dependencies from the object graph, and other dependencies from a caller at runtime. It's like a Provider that takes parameters.

public class ImageDownloader {
  // Get these dependencies from the injector.
  private final HttpClient httpClient;
  private final ExecutorService executorService;

  // Get these from the caller.
  private final URL imageUrl;
  private final ImageCallback callback;

  ...
}

PRIOR ART

NO ASSISTANCE

It isn't that difficult though it is verbose. Fewer fields can be final, and it's possible to forget to call a setter. The fact that you need to inject a Provider is ugly, and looks magical.

public class PhotoGallery implements ImageCallback {
  @Inject Provider<ImageDownloader> imageDownloaderProvider;

  public void showImage(URL imageUrl) {
    ImageDownloader downloader = imageDownloaderProvider.get();
    downloader.setImageUrl(imageUrl);
    downloader.setCallback(this));
    downloader.download();
  }

  ...
}

DIY WITH PROVIDER METHOD

This is what we're doing today at Square. We write the factory interface that we want:

public class ImageDownloader {
  ...
  public interface Factory {
    ImageDownloader create(URL imageUrl, ImageCallback callback);
  }
}

And we also write a big fat method to implement the factory:

public class ImageModule {
  ...
  @Provides public ImageModule.Factory(
      final Provider<HttpClient> httpClientProvider, final Provider<ExecutorService> executorServiceProvider) {
    return new ImageDownloader.Factory() {
      public ImageDownloader create(URL imageUrl, ImageCallback callback) {
        return new ImageDownloader(httpClientProvider.get(), executorServiceProvider.get(),
            imageUrl, callback);
      }
  }
}

It's verbose, but not too verbose. My biggest problem with this approach is that it requires providers in the parameters, otherwise a factory that's used twice may behave strangely. In this case the two ImageDownloaders could share an HttpClient, even if that class isn't singleton.

This solution isn't very discoverable. It's a design pattern that you may not come up with on your own.

GUICE

Guice has an extension AssistedInject to solve this problem. They let you define a custom Factory interface that replaces Provider. You use @Assisted annotations to match factory method parameters to the corresponding injections, plus a clumsy FactoryModuleBuilder thing to register it all.

install(new FactoryModuleBuilder()
     .implement(Payment.class, RealPayment.class)
     .build(PaymentFactory.class));

I like the factory interface. The @Assisted is tolerable, and the FactoryModuleBuilder is quite unfortunate.

JESSE'S PROPOSAL

I want the Factory interface most of all. Dagger needs to recognize a Factory at an injection site, and implement it as we did above in the DIY solution. Basically it should work exactly like the DIY solution, only you don't need to write any @Provides method.

RECOGNIZING FACTORIES

Probably via an annotation:

public class ImageDownloader {
  ...
  @AssistedFactory
  public interface Factory {
    ImageDownloader create(URL imageUrl, ImageCallback callback);
  }
}

FACTORY RETURN TYPES

It's possible that we want to return say, ImageDownloaderImpl instead of ImageDownloader. All the APIs I imagine for this are horrible, and I don't think it's that useful. If you need it, use the DIY solution.

MATCHING PARAMETERS

We probably don't need very much here. We don't need @Assisted. In the unlikely event that two factory method parameters have the same type, just use @Named to differentiate them.

IMPLEMENTATION

I'm tempted to use ObjectGraph.plus(), since it means we can use the existing code to inject the target object. Here's a sketch of code we could generate:

@Module(complete=false)
public class ImageDownloader$Factory$Module {
  @Provides public ImageDownloader.Factory provideFactory(ObjectGraph base) {
    return new ImageDownloader.Factory() {
      ImageDownloader create(URL imageUrl, ImageCallback callback) {
        ObjectGraph newGraph = base.plus(new ParametersModule(imageUrl, callback));
        return newGraph.get(ImageDownloader.class);
      }

    };
  }

  @Module(entryPoints=ImageDownloader.class, complete=false);
  static class ParametersModule {
    private final URL imageUrl;
    private final ImageCallback callback;
    ...
    @Provides public URL provideImageUrl() {

      return imageUrl;
    }
    @Provides public ImageCallback provideCallback() {
      return callback;
    }
  }
}

NEXT STEPS

Let's nail down the API. Remember, Dagger's raison d'être is its small size. If you want fancy features, use Guice.

[Christian Gruber]

Alternate , though similar, proposal

https://docs.google.com/document/d/1BioRZqm-pmhAPkFcKu_DD7Xslzw5bTx4ls00x0xCjSQ/view

It's not entirely dissimilar, though you mark the value type with an annotation that indicates what class is the factory interface, and two things are generated - a factory implementation that conforms to the factory interface and weaves together call-stack parameters and injected things, and a specialized InjectAdapter that links the factory interface to the implementation, to avoid needing to muck about with @Provides or module stuff - the loader will just load it, and the full graph analysis will support the case where an interface type has a loadable adapter.

It has the distinct advantage of being 90% implemented in a branch on my client.

Reading your proposal, I think I can make it so you don't even need @Parameter (@Assisted) annotations, but I left them in so one could have a value type that did "field injection" of parameters and to disambiguate quickly in the constructor which fields are injected. But it could be done without them.

Christian.

On 24 Jan 2013, at 19:35, Jesse Wilson wrote:

There's been a lot https://github.com/square/dagger/pull/148 of
interesthttps://plus.google.com/106927208431194134382/posts/DuVu1VjgeUwin
doing https://twitter.com/tbroyer/status/250636720234700800 assisted

injection for Dagger. Let's all agree on the problem, propose some
solutions, and decide whether that solution is worth its cost.
THE PROBLEM

I have a class that gets some dependencies from the object graph, and other
dependencies from a caller at runtime. It's like a Provider that takes
parameters.

public class ImageDownloader {
// Get these dependencies from the injector.
private final HttpClient httpClient;
private final ExecutorService executorService;

// Get these from the caller.
private final URL imageUrl;
private final ImageCallback callback;

...
}

PRIOR ART NO ASSISTANCE

It isn't that difficult though it is verbose. Fewer fields can be final,
and it's possible to forget to call a setter. The fact that you need to
inject a Provider is ugly, and looks magical.

public class PhotoGallery implements ImageCallback {

@Inject Provider imageDownloaderProvider;

public void showImage(URL imageUrl) {
ImageDownloader downloader = imageDownloaderProvider.get();
downloader.setImageUrl(imageUrl);
downloader.setCallback(this));
downloader.download();
}

...
}

DIY WITH PROVIDER METHOD

This is what we're doing today at Square. We write the factory interface
that we want:

public class ImageDownloader {
...
public interface Factory {
ImageDownloader create(URL imageUrl, ImageCallback callback);
}
}

And we also write a big fat method to implement the factory:

public class ImageModule {
...
@Provides public ImageModule.Factory(

final Provider httpClientProvider, final
Provider executorServiceProvider) {

return new ImageDownloader.Factory() {
public ImageDownloader create(URL imageUrl, ImageCallback callback) {
return new ImageDownloader(httpClientProvider.get(),
executorServiceProvider.get(),
imageUrl, callback);
}
}
}

It's verbose, but not too verbose. My biggest problem with this approach is
that it requires providers in the parameters, otherwise a factory that's
used twice may behave strangely. In this case the two ImageDownloaders
could share an HttpClient, even if that class isn't singleton.

This solution isn't very discoverable. It's a design pattern that you may
not come up with on your own.
GUICE

Guice has an extension
AssistedInjecthttps://code.google.com/p/google-guice/wiki/AssistedInjectto

---

You received this message because you are subscribed to the Google Groups "Dagger Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to dagger-discus...@googlegroups.com.
To post to this group, send email to dagger-...@googlegroups.com.

[Jesse Wilson]

Nice doc. I like that we're mostly on the same page. Assuming code gen is disabled, how would Dagger discover that a Factory serves assisted injection?

import java.util.concurrent.ThreadFactory;

class Foo {
  @Inject ThreadFactory threadFactory;
}

Does it look at the return type of ThreadFactory.newThread() ?

I don' think it's a popular feature, but with Guice's assisted injection it's possible to use one factory to construct multiple types:

interface Factory {
  ImageDownloader newImageDownloader(URL url, Callback callback);
  ImageUploader newImageUploader(URL url, Bitmap bitmap);
}

[Christian Gruber]

So, this notion of "if code generation is disabled" is a curious one. :D

Do we actually have a case where we want to support a reflection-only, won't load an adapter even if it's there?

The current implementation I have noodling around doesn't support reflection, and that's so that we can support the full loss of parameter name information in the class file, which you can do in annotation processing, but not with reflection. I know the recent implementation proposal added an annotation parameter to the @Assisted annotation, precisely to work around all of this. I had really hoped to avoid having @Assisted("foo") because of the verbosity of:

public Foobar(@Assisted("myLongStringName") String myLongStringName)

In general, I want to reduce duplication of semantic information where we can avoid it.

But this means that a factory implementation would only be useable without any further effort in a code-gen/loading situation.

That said, if you generated the factories, but not the adapters (not the current setup, but totally doable) then it would be trivial to do:

@Provides Foo.Factory blah(Foo.Factory.FactoryImpl impl) { return impl; }

… and that would just work. But in order for this to be even meaningful would be to separate the auto factory stuff into a separate project, since if you are already using the code-gen project, you might as well use the code-gen, right?

As to multiple factories, I think it's not an incredible savings to limit the factory to one create method. It's not a necessary limitation, though it does make things slightly easier. It's just what we choose to generate. I think it complicates the API a bit, in that it becomes more of a factory interface.

One further thought THe other thing I use @Parameter for is to make sure that I exclude it from the general adapter, so that an InjectAdapter isn't generated if it's a factory-built class. That is pretty crucial, because otherwise we'll generate adapters naively for anything with an @Inject class on it, and fail with errors if it doesn't have a suitable JIT-able constructor.

Christian.

[Christian Gruber]

Oh, and use the edit link, not the view link, if you want to comment
(this is for anyone on the list, not just jesse).

https://docs.google.com/document/d/1BioRZqm-pmhAPkFcKu_DD7Xslzw5bTx4ls00x0xCjSQ/edit

Christian.

On 24 Jan 2013, at 20:16, Jesse Wilson wrote:

> --

[Jesse Wilson]

On Thu, Jan 24, 2013 at 8:50 PM, Christian Gruber <cgr...@google.com> wrote:

Do we actually have a case where we want to support a reflection-only, won't load an adapter even if it's there?

Dagger needs to work without dagger-compiler. Code generation is clumsy. At Square our development builds don't use code generation.

One further thought THe other thing I use @Parameter for is to make sure that I exclude it from the general adapter, so that an InjectAdapter isn't generated if it's a factory-built class. That is pretty crucial, because otherwise we'll generate adapters naively for anything with an @Inject class on it, and fail with errors if it doesn't have a suitable JIT-able constructor.

I still think using ObjectGraph.plus() is the way to go. It avoids the need for @Parameter altogether.

[Christian Gruber]

On 24 Jan 2013, at 21:17, Jesse Wilson wrote:

> On Thu, Jan 24, 2013 at 8:50 PM, Christian Gruber <cgr...@google.com>
> wrote:
>
> Do we actually have a case where we want to support a reflection-only,
>> won't load an adapter even if it's there?
>>
> Dagger needs to work without dagger-compiler. Code generation is
> clumsy.
> At Square our development builds don't use code generation.

So, if the Factories are generated, then the actual factories can be
loaded without the use of dagger-compiler at run-time. That said, I was
looking at how to go about creating a run-time version. As to clumsy,
I'm not sure why you think it's so. I've been using it in development
consistently now when I've been working with client code, even in
development. I'm a bit confused.

> One further thought THe other thing I use @Parameter for is to make
> sure
>> that I exclude it from the general adapter, so that an InjectAdapter
>> isn't
>> generated if it's a factory-built class. That is pretty crucial,
>> because
>> otherwise we'll generate adapters naively for anything with an
>> @Inject
>> class on it, and fail with errors if it doesn't have a suitable
>> JIT-able
>> constructor.
>>
> I still think using ObjectGraph.plus() is the way to go. It avoids the
> need for @Parameter altogether.
>

I don't like it. ObjectGraph.plus() is really a narrower-lifetime
scoping concept, and I think going this way makes for a confusing API.
Maybe I'm missing some of how you imagine this implemented. How does it
avoid the need for @Parameter (Note, I can avoid it in code-gen too, I
just think it's easier without to implement some things).

Christian.

[Thomas Broyer]

On Friday, January 25, 2013 1:35:44 AM UTC+1, Jesse Wilson wrote:

There's been a lot of interest in doing assisted injection for Dagger.

Wow! You dug far enough in Twitter history to find my tweet back from 4 months ago? ;-)

How about 3rd-party factories? (e.g. java.util.concurrent.ThreadFactory that you used later in this thread in an example)

There should probably be a @Factories annotation to put on modules, or a 'Class<?>[] factories()' value in the @Module annotation to complement that @AssistedFactory annotation (@Factory might be a better name though).

 

FACTORY RETURN TYPES

It's possible that we want to return say, ImageDownloaderImpl instead of ImageDownloader. All the APIs I imagine for this are horrible, and I don't think it's that useful. If you need it, use the DIY solution.

Or just define your own interface inheriting the one you want and use a @Provides method to do the "cast":

   interface ImageDownloaderImplFactory extends ImageDownloader.Factory {

     @Override ImageDownloaderImpl create(URL imageUrl, ImageCallback callback);

   }

   @Provides ImageDownloader.Factory imageDownloaderFactory(ImageDownloaderImplFactory factory) {

     return factory;

   }

 

MATCHING PARAMETERS

We probably don't need very much here. We don't need @Assisted. In the unlikely event that two factory method parameters have the same type, just use @Named to differentiate them.

+1

(even though I like the explicitness of @Assisted or @Param)

 

IMPLEMENTATION

I'm tempted to use ObjectGraph.plus(), since it means we can use the existing code to inject the target object. Here's a sketch of code we could generate:

@Module(complete=false)

Shouldn't it be complete=true and addsTo=Xxx.class instead?

(even if that means generating an ImageDownloader$Factory$Module$Xxx class, for each module that uses the factory)

 

public class ImageDownloader$Factory$Module {
  @Provides public ImageDownloader.Factory provideFactory(ObjectGraph base) {
    return new ImageDownloader.Factory() {
      ImageDownloader create(URL imageUrl, ImageCallback callback) {
        ObjectGraph newGraph = base.plus(new ParametersModule(imageUrl, callback));
        return newGraph.get(ImageDownloader.class);
      }

    };
  }

  @Module(entryPoints=ImageDownloader.class, complete=false);
  static class ParametersModule {
    private final URL imageUrl;
    private final ImageCallback callback;
    ...
    @Provides public URL provideImageUrl() {

      return imageUrl;
    }
    @Provides public ImageCallback provideCallback() {
      return callback;
    }
  }
}

The only problem I see with ObjectGraph.plus() is that it makes it more complicated to use with GWT (it would need to be reimplemented in the GWT generator using codegen similar to Christian's proposal).

That said, I haven't worked on Sheath for a while and it's not necessarily worth it to consider it here (reimplementing the codegen in the GWT generator is no big deal).

 

NEXT STEPS

Let's nail down the API. Remember, Dagger's raison d'être is its small size. If you want fancy features, use Guice.

Below, another comment from you on the same thread:

I don' think it's a popular feature, but with Guice's assisted injection it's possible to use one factory to construct multiple types:

interface Factory {
  ImageDownloader newImageDownloader(URL url, Callback callback);
  ImageUploader newImageUploader(URL url, Bitmap bitmap);
}

I use it a lot in GWT client code (with GIN), and it shouldn't be a problem to implement either AFAICT.

[Jesse Wilson]

On Thu, Jan 24, 2013 at 9:38 PM, Christian Gruber <christiane...@gmail.com> wrote:

I've been using it in development consistently now when I've been working with client code, even in development. 

One example: Dagger's code generator is broken for incremental builds. If it generates Foo$InjectAdapter.java and then you rename Foo.java to Bar.java, nothing garbage collects the old inject adapter. To work around this, you need to do clean builds.

ObjectGraph.plus() is really a narrower-lifetime scoping concept,

Yes, and these are narrower-lifetime objects. Otherwise we wouldn't need local variables to construct 'em.

I think going this way makes for a confusing API.

Yeah, the entire premise of assisted injection is confusing. I fear it's a power users feature.

[Thomas Broyer]

On Friday, January 25, 2013 3:54:40 AM UTC+1, Jesse Wilson wrote:

On Thu, Jan 24, 2013 at 9:38 PM, Christian Gruber <christiane...@gmail.com> wrote:

I've been using it in development consistently now when I've been working with client code, even in development. 

One example: Dagger's code generator is broken for incremental builds. If it generates Foo$InjectAdapter.java and then you rename Foo.java to Bar.java, nothing garbage collects the old inject adapter. To work around this, you need to do clean builds.

This is a compiler/build-system issue.

If something garbage-collects Foo.class, then it's probably using heuristics (SCons does that) and will indeed miss files generated by annotation processors.

Any tool that directly calls the Java compiler in-process (maven-compiler-plugin, SBT, Gradle) has the tools to garbage-collect both Foo.class and Foo$InjectAdapter.class (javax.tools.JavaFileManager for instance will give you the exact set of generated files so if you track the previous set and new set, you can infer the files that can be garbage collected) It's not an easy task though, and nobody actually does it AFAIK (not even Eclipse, but I must admit I haven't looked at what IntelliJ does). On a related subject, see http://blog.lexspoon.org/2012/12/recursive-maven-considered-harmful.html and the discussion I spawned in the Maven Users list –MarkMail URL in the comments–

 

ObjectGraph.plus() is really a narrower-lifetime scoping concept,

Yes, and these are narrower-lifetime objects. Otherwise we wouldn't need local variables to construct 'em.

They're contextual, not necessarily narrower-lifetime objects.

[Christian Gruber]

On 24 Jan 2013, at 19:35, Jesse Wilson wrote:

> public class ImageDownloader {
> ...
> @AssistedFactory
> public interface Factory {
> ImageDownloader create(URL imageUrl, ImageCallback callback);
> }
> }
>

I like this - I went the opposite way, indicating on the value class
what the factory will be, but I think we can go the other way. But
inferring from the return type works, I think.

> MATCHING PARAMETERS
>
> We probably don't need very much here. We don't need @Assisted. In the
> unlikely event that two factory method parameters have the same type,
> just
> use @Named to differentiate them.

Actually, it's quite common for factory parameters to be the same type,
and I was hoping to avoid @Named or any other parameter that uses a
value() if at all possible.

> IMPLEMENTATION
>
> I'm tempted to use ObjectGraph.plus(), since it means we can use the
> existing code to inject the target object. Here's a sketch of code we
> could
> generate:
>
> @Module(complete=false)
> public class ImageDownloader$Factory$Module {
> @Provides public ImageDownloader.Factory provideFactory(ObjectGraph
> base) {
> return new ImageDownloader.Factory() {
> ImageDownloader create(URL imageUrl, ImageCallback callback) {
> ObjectGraph newGraph = base.plus(new
> ParametersModule(imageUrl, callback));
> return newGraph.get(ImageDownloader.class);
> }
>
> };
> }

I dislike injecting the graph, extremely much. It hurts, conceptually,
though I do understand why you would want to do it, especially given
your orientation towards generating modules in this process. But it
also forces linking on everything in the graph even if such things
aren't used in the production of the factory. I don't think we need to
generate modules, and I'd like to avoid it. I think we can generate (or
reflectively use) Bindings that know how to handle these things. My
proposal does the above, though it is lacking on the reflective side,
and the API I went for is tilted in favor of the code-gen case because
it makes the end-user API much simpler and cleaner.

>
> @Module(entryPoints=ImageDownloader.class, complete=false);
> static class ParametersModule {
> private final URL imageUrl;
> private final ImageCallback callback;
> ...
> @Provides public URL provideImageUrl() {
>
> return imageUrl;
> }
> @Provides public ImageCallback provideCallback() {
> return callback;
> }
> }
> }

Yeah - really not a fan of generating modules in this context.

Christian.

[Christian Gruber]

On 24 Jan 2013, at 22:11, Thomas Broyer wrote:

> On Friday, January 25, 2013 3:54:40 AM UTC+1, Jesse Wilson wrote:
>>
>> On Thu, Jan 24, 2013 at 9:38 PM, Christian Gruber

>> <christiane...@gmail.com<javascript:>>

Agreed. I don't want our design in Dagger to be driven by faults in the
build tools. Frankly, I'm ok doing a clean build on code that's
generated from time to time if I do something that needs it. But I
don't want to design to the tools.

>> ObjectGraph.plus() is really a narrower-lifetime scoping concept,
>>>
>> Yes, and these are narrower-lifetime objects. Otherwise we wouldn't
>> need
>> local variables to construct 'em.
>>
>

> They're *contextual*, not necessarily narrower-lifetime objects.
>

Amen.

Christian.

[Jesse Wilson]

On Thu, Jan 24, 2013 at 10:12 PM, Christian Gruber <christiane...@gmail.com> wrote:

Actually, it's quite common for factory parameters to be the same type, and I was hoping to avoid @Named or any other parameter that uses a value() if at all possible.

What would you use instead? Requiring code generation is a nonstarter.

...because it makes the end-user API much simpler and cleaner.

Please expand on this? Perhaps a programmer mistake that is eliminated by this API? For example, one problem with proposed API is that it uses @Inject but unlike most classes with this annotation, it cannot be injected directly; it needs a factory.

[Christian Gruber]

On 24 Jan 2013, at 22:41, Jesse Wilson wrote:

On Thu, Jan 24, 2013 at 10:12 PM, Christian Gruber christiane...@gmail.com wrote:

Actually, it's quite common for factory parameters to be the same type, and
I was hoping to avoid @Named or any other parameter that uses a value() if
at all possible.

What would you use instead? Requiring code generation is a nonstarter.

…because it makes the end-user API much simpler and cleaner.

Please expand on this? Perhaps a programmer mistake that is eliminated by
this API? For example, one problem with proposed API is that it uses
@Inject but unlike most classes with this annotation, it cannot be
injected directly; it needs a factory.

@Inject is used, but @Inject doesn't only imply that it can be injected directly. It means that certain members will be injected with dependencies, or that the marked constructor will be used to inject - but we already have cases of partially injected objects. Activities, and other things that don't follow the normal lifecycle use @Inject and are injected, but not all fields are injected, only marked ones. Dagger is partially involved, but not wholly involved in its creation/preparation.

So too with "assisted injection." I don't think using @Inject to signal that certain ingredients need to come from the graph is even slightly misleading.

What I mean by simpler and cleaner was more about it being uncluttered, however. Just having @Named("foo") annotations on parameters is pretty frustrating to read and organize. But that's an aesthetic thing.

One option is to simply say that there is no field injection on value types, and that the constructor takes N in-order parameters. On the implementation type, you mark those parameters that are to be satisfied by the injector, and all other are satisfyied, in-order, from the factory interface.

Something like

class Foo {
  Foo(A a, @Graph B b, C c, @Graph D d) { … }

  @Factory
  static interface Factory {
    Foo make(Aa, Cc);
  }
}

That can be investigated reflectively and things positionally inferred. It could even allow for Graph-injected fields, but wouldn't use the @Inject annotation, so wouldn't be mistaken for injectables. That might look like:

class Foo {
  @Graph E e;
  Foo(A a, @Graph B b, C c, @Graph D d) { … }

  @Factory
  static interface Factory {
    Foo make(Aa, Cc);
  }
}

That's clean and simple, is consistent with the code-gen approach I was taking, but is also workable with reflection, requires no extra parameters on the annotations, and is pretty hard to misinterpret. Could go the other way and mark parameters instead of graph-injection sites, but I think this way is better, though it would probably do to think carefully about the right name. We want to associate it mentally with the graph, but not in a way that confuses it with the JSR-330 stuff.

Thoughts?

cheers,
Christian.

[Jesse Wilson]

I'd rather use @Named(), which is more explicit than parameter order. I don't like @Graph, especially since we don't use it elsewhere.

[Christian Gruber]

On 25 Jan 2013, at 0:00, Jesse Wilson wrote:

> I'd rather use @Named(), which is more explicit than parameter order. I
> don't like @Graph, especially since we don't use it elsewhere.

@Named to provide precisely what semantics?

[Christian Gruber]

Can you re-write my example code using @Named as you would? I just want
to see it to get a feel for it.

[Jesse Wilson]

A, B and C are from the graph. The factory provides two strings. You don't need @Named; any qualifier annotation would work.

public class Foo {
  @Inject A a;

  @Inject Foo(@Named("zip") String zip, @Named("city") String city, B b, C c) {
    ...
  }

  @AssistedFactory
  interface Factory {
    Foo make(@Named("city") String city, @Named("zip") String zip);
  }
}

[Pierre-Yves Ricau]

Although I agree there's semantic duplication, this still seems quite clear and simpler then without @AssistedFactory

Just read this: "Dagger's code generator is broken for incremental builds. If it generates Foo$InjectAdapter.java and then you rename Foo.javato Bar.java, nothing garbage collects the old inject adapter."

AFAIK, it's the purpose of the "originatingElements" vararg parameter in "Filer.createSourceFile()" to enable this garbage collection. Used here for dagger. The old inject adapter should normally be removed, I've seen the "refactor => gc" action work on other annotation processing projects, at least with Eclipse.

2013/1/25 Jesse Wilson <je...@swank.ca>

--

You received this message because you are subscribed to the Google Groups "Dagger Discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to dagger-discus...@googlegroups.com.

To post to this group, send email to dagger-...@googlegroups.com.
 
 

--
Pierre-Yves Ricau

http://www.piwai.info

[Yury Levin]

I mean ONLY generating factory without implementation in ReflectivePlugin, sorry.

пятница, 25 января 2013 г., 13:51:03 UTC+4 пользователь Yury Levin написал:

Make @Assisted not qualified more. It's really my bad. Using @Named for naming assisted injections is better.

https://github.com/FinamTrade/dagger/compare/master...assisted-injections

But generating factory is make some problem. I believe that It need to working with ReflectivePlugin without code generation. 

Second, when we annotated class with assisted injections it's more comfortable to use @Inject annotation. But InjectAdapted for this class should not be generated in this case, it's redundant.

Third, I think that all dependencies should be define in Module. But generating custom Module looks like hack.

пятница, 25 января 2013 г., 4:35:44 UTC+4 пользователь Jesse Wilson написал:

[Jeff Axelrod]

Writing from my phone to quickly add my two cents. Jesse, dagger's ppeal for me is exclusively its code generation. On android, guice is unacceptably slow. I assumed this was why you guys at square started dagger.

I use assisted injection all over my code. Also if you can, take a look at Fred Faber's legprovider inside gin for an elegant robot legs problem solution.

[Christian Gruber]

Ok… but this makes for even more duplication in names all over the place - in short, clutter. But that's aesthetic on my part.

The big showstopper here, for me, is that this isn't an injectable class, but you're using @INject in a way that is misleadingly familiar, and you're using @Named in a way that is the INVERSE of how it's used in normal injectable classes. That is, this class looks like you are injecting @Named("zip") String zip, etc.

@Named is a qualifier in the JSR-330… it has a specific meaning for injection and you're using it for the non-injected case. In my own examples, I was at least using @Inject normally.

What happens if there is an @Named("zip")/String bound in the graph? Obviously it wouldn't be injected, because this is factory managed, but it would read as if it should. We also need a signal to Dagger on the value class itself that this is NOT an injectable class. Otherwise Foo would be injected with the dependencies A, B, C, zip/String, city/String regardless, and it would be massively inefficient to scan for all possible factories and test for membership before attempting to inject.

Lastly, I think having a non-JSR-330 annotation for this, something that cannot be confused with injection is crucial for readability. It's not a Named type key, which is what qualifiers are about - it's a factory-parameter call-stack dependency. Not the same concept, not the same label.

Consider yours:

public class Foo {
  @Inject A a;

  @Inject Foo(@Named("zip") String zip, @Named("city") String city, B b, C c) {
    ...
  }

  @AssistedFactory
  interface Factory {
    Foo make(@Named("city") String city, @Named("zip") String zip);
  }
}

In light of an alternative

public class Foo {
  @Inject A a;

  @Inject Foo(@Param("zip") String zip, @Param("city") String city, B b, C c) {
    ...
  }

  @Factory
  interface Factory {
    Foo make(@Param("city") String city, @Param("zip")  String zip);
  }
}

This makes the annotations @Inject and @Named (if needed) have the same semantics, but the @Param is the signal that this is not directly injectable, but only partially. This would result in no attempt to JIT bind by reflection or generate an adapter, and all the signals are there to generate a factory properly.

That said, I still think this is cleaner.

public class Foo {
  @Inject A a;

  @Inject Foo(@Param String city, @Param String zip, B b, C c) {
    ...
  }

  @Factory
  interface Factory {
    Foo make(String city, String zip);
  }
}

And as long as you assert order as the default interpretation absent the additional signal (we can make the disambiguation available, but not necessary), then it's fine. The generator can catch mistakes of this sort:

public class Foo {
  @Inject A a;

  @Inject Foo(@Param String zip, @Param String city, B b, C c) {
    ...
  }

  @Factory
  interface Factory {
    Foo make(String city, String zip);
  }
}

where you have two strings in a row, and you invert them. Or we can simply say, if you have duplicated types so there's ambiguity, then it's an error if you don't have disambiguated, labelled parameters. I'd prefer an in-order interpretation, though. After all, that's how Java works - parameters are positional. And this makes what I think is the simplest, most readable code. And it doesn't require code generation, though you get more benefits if you use such.

[Christian Gruber]

Don't worry, Jeff. Jesse is not saying we won't have code-gen - he's
saying we must ALSO support a reflection back-end for this feature. I
agree it would be good, though I was more comfortable than Jesse with
leaving it generator-only. I think we can get to an API that can be
supported in both cases, and then if you want you can leave off
reflection.

Side note, I'm going to push for a GWT compatible support in the next
couple of weeks in order to support some early adopters in Google. That
version will support all features without reflection, out of necessity.

Christian.

[Jesse Wilson]

Nitpick:

  @Factory
  interface Factory {
    Foo make(@Param("city") String city, @Param("zip")  String zip);
  }

You can use Factory as the name of the annotation, or you can use Factory as the name of the interface. You can't, unfortunately, use Factory for both.

[Christian Gruber]

On 25 Jan 2013, at 10:31, Jesse Wilson wrote:

Nitpick:

@Factory
interface Factory {
Foo make(@Param("city") String city, @Param("zip") String zip);
}

You can use Factory as the name of the annotation, or you can use

Factoryas the name of the interface. You can't, unfortunately, use
Factory for both.

Heh. Whoops. True. @AutoFactory? I don't like the word assist, mostly just because what is assisting what is ambiguous. Is it a factory that is assisted, or a factory assisting the injection. It's not THAT big a deal - I just wanted a less ambiguous word.

Alternately...

@dagger.Factory
interface Factory {
  Foo make(@Param("city") String city, @Param("zip")  String zip);
}

:)

Christian.

[Thomas Broyer]

Fortunately, Dagger has short FQCNs:

  @dagger.Factory
  interface Factory {

 

[Yury Levin]

What if I have factory for interface or abstract class instead final class in my code and I want to bind it to realization in module? In our project we use this ability of Guice and now it need to support in Dagger. I believe it's usable feature.

пятница, 25 января 2013 г., 4:53:54 UTC+4 пользователь Christian Edward Gruber написал:

[Christian Gruber]

Could you not just bind the class normally?

```
interface Foo {
String blah();
static final class Factory {
public Foo create(final String blah) {
return new Foo() {
@Override blah() { return blah; }
}
}
}
}
```

you would just make a module and do

```
@Provides Foo.Factory fooFactory() {
return new Foo.Factory(); // or however you create it.
}
```

I don't see what feature is needed if you have a static final factory
class. Just bind the class. This feature is for automatically
generating factories from an interface.

Christian.

>> solution. Basically it should work *exactly* like the DIY solution,

>> ------------------------------

>>
>> You received this message because you are subscribed to the Google
>> Groups
>> "Dagger Discuss" group.
>> To unsubscribe from this group and stop receiving emails from it,
>> send an

>> email to dagger-discus...@googlegroups.com <javascript:>.

>> To post to this group, send email to

>> dagger-...@googlegroups.com<javascript:>
>> .
>>
>>
>
> --

> You received this message because you are subscribed to the Google
> Groups "Dagger Discuss" group.

> To post to this group, send email to dagger-...@googlegroups.com.

> To unsubscribe from this group, send email to
> dagger-discus...@googlegroups.com.

[Yury Levin]

Do I understand correctly?

``` 
interface Foo { 
   String blah(); 
} 
```

 

```

interface FooFactory {

   Foo provideFoo();

}

```

Implementation:

```

@FactoryManaged(Factory.class)

class MyFoo implements Foo {

  public String blah() {

     return "Bar";

  }

  interface Factory extends FooFactory { 

  }

}

``` 

Module:

```

@Provides FooFactory fooFactory() { 
   return new MyFoo.Factory();
} 
``` 

Will it work?

суббота, 26 января 2013 г., 3:32:48 UTC+4 пользователь Christian Edward Gruber написал:

[Yury Levin]

Proposed API for Assisted Injection

https://docs.google.com/document/d/1lPbDBK-5RDA1a8iR2pEeqwGEoyPAK0NXFVmQrD0Y0DQ/edit

пятница, 25 января 2013 г., 4:35:44 UTC+4 пользователь Jesse Wilson написал:

There's been a lot of interest in doing assisted injection for Dagger. Let's all agree on the problem, propose some solutions, and decide whether that solution is worth its cost.

THE PROBLEM

I have a class that gets some dependencies from the object graph, and other dependencies from a caller at runtime. It's like a Provider that takes parameters.

public class ImageDownloader {
  // Get these dependencies from the injector.
  private final HttpClient httpClient;
  private final ExecutorService executorService;

  // Get these from the caller.
  private final URL imageUrl;
  private final ImageCallback callback;

  ...
}

PRIOR ART

NO ASSISTANCE

It isn't that difficult though it is verbose. Fewer fields can be final, and it's possible to forget to call a setter. The fact that you need to inject a Provider is ugly, and looks magical.

public class PhotoGallery implements ImageCallback {

  @Inject Provider<ImageDownloader> imageDownloaderProvider;

  public void showImage(URL imageUrl) {
    ImageDownloader downloader = imageDownloaderProvider.get();
    downloader.setImageUrl(imageUrl);
    downloader.setCallback(this));
    downloader.download();
  }

  ...
}

DIY WITH PROVIDER METHOD

This is what we're doing today at Square. We write the factory interface that we want:

public class ImageDownloader {
  ...
  public interface Factory {
    ImageDownloader create(URL imageUrl, ImageCallback callback);
  }
}

And we also write a big fat method to implement the factory:

public class ImageModule {
  ...
  @Provides public

 ImageModule.Factory(
      final Provider<HttpClient> httpClientProvider, final Provider<ExecutorService> executorServiceProvider) {
    return new ImageDownloader.Factory() {
      public ImageDownloader create(URL imageUrl, ImageCallback callback) {
        return new ImageDownloader(httpClientProvider.get(), executorServiceProvider.get(),
            imageUrl, callback);
      }
  }
}

It's verbose, but not too verbose. My biggest problem with this approach is that it requires providers in the parameters, otherwise a factory that's used twice may behave strangely. In this case the two ImageDownloaders could share an HttpClient, even if that class isn't singleton.

This solution isn't very discoverable. It's a design pattern that you may not come up with on your own.

GUICE

Guice has an extension AssistedInject to solve this problem. They let you define a custom Factory interface that replaces Provider. You use @Assisted annotations to match factory method parameters to the corresponding injections, plus a clumsy FactoryModuleBuilder thing to register it all.

install(new FactoryModuleBuilder()
     .implement(Payment.class, RealPayment.class)
     .build(PaymentFactory.class));

I like the factory interface. The @Assisted is tolerable, and the FactoryModuleBuilder is quite unfortunate.

JESSE'S PROPOSAL

I want the Factory interface most of all. Dagger needs to recognize a Factory at an injection site, and implement it as we did above in the DIY solution. Basically it should work exactly like the DIY solution, only you don't need to write any @Provides method.

[Thomas Broyer]

On Friday, January 25, 2013 1:35:44 AM UTC+1, Jesse Wilson wrote:

There's been a lot of interest in doing assisted injection for Dagger. Let's all agree on the problem, propose some solutions, and decide whether that solution is worth its cost.

THE PROBLEM

I have a class that gets some dependencies from the object graph, and other dependencies from a caller at runtime. It's like a Provider that takes parameters.

public class ImageDownloader {
  // Get these dependencies from the injector.
  private final HttpClient httpClient;
  private final ExecutorService executorService;

  // Get these from the caller.
  private final URL imageUrl;
  private final ImageCallback callback;

  ...
}

Back to basics, which use cases do we want to support? and to which extent? (i.e. would there be easy workaround?)

• binding the factory to a sub-type of the declared return type (the Payment→RealPayment from Guice's wiki: https://code.google.com/p/google-guice/wiki/AssistedInject)
• a factory interface from a 3rd-party library (e.g. java.util.concurrent.ThreadFactory)
• a class that is injectable but, for some use case, we want to provide some dependencies through factory arguments (the class could possibly be injected directly elsewhere in the app)
• several methods in the factory with the same return type, dispatching to the appropriate constructor/members of the created class (the “Multiple factory methods for the same type” case from GUice's javadoc: http://google-guice.googlecode.com/svn/trunk/javadoc/com/google/inject/assistedinject/FactoryModuleBuilder.html)
• several methods in the factory with the same return type, being able to map them to different sub-types (the “More configuration options”, with fast-car → Porsche & clean-car → Prius, from Guice's javadoc: http://google-guice.googlecode.com/svn/trunk/javadoc/com/google/inject/assistedinject/FactoryModuleBuilder.html)
• should the assisted parameters be injectable to fields (and methods when/if we support them) or only as constructor arguments?

Once we settled on the use-cases:

• how do we want to disambiguate arguments of the same type?

[Christian Gruber]

Good pull-back.

I have a work-in-progress candidate implementation. It hasn't solved the disambiguation question yet, as I wanted to consider the different ways. I've updated my doc to reflect some changes from this thread.

That said, some thoughts on each. One meta-question - how frequent are these use-cases? With the odd exception, I don't have strong enough feeling for others' usage to really push for any of these one way or another.

On 28 Jan 2013, at 14:11, Thomas Broyer wrote:

Back to basics, which use cases do we want to support? and to which
extent? (i.e. would there be easy workaround?)

• binding the factory to a sub-type of the declared return type (the Payment→RealPayment from Guice's wiki: https://code.google.com/p/google-guice/wiki/AssistedInject)

I think this is tricky, but worth considering. The main issue here is signaling.

• a factory interface from a 3rd-party library (e.g. java.util.concurrent.ThreadFactory)

This feels like it is best handled with an @Provides method, but it could be done.

• a class that is injectable but, for some use case, we want to provide some dependencies through factory arguments (the class could possibly be injected directly elsewhere in the app)

Hmm. This one is tricky, because of how to "override" the naked class description and indicate to Dagger that it needs to manage with a factory.

The above three actually all seem like we may need two mechanisms for specifying factories. Firstly, a mechanism indicating that a value type is intended to be managed by a factory with some injectable pieces, by flagging these in the source somewhere. And secondly, a way to tell Dagger without any access to the source that a given type is factory managed, and should be treated as such, along with what parts of it are injectable and what parts are part of the factory method parameter.

• several methods in the factory with the same return type, dispatching to the appropriate constructor/members of the created class (the “Multiple factory methods for the same type” case from GUice's javadoc: http://google-guice.googlecode.com/svn/trunk/javadoc/com/google/inject/assistedinject/FactoryModuleBuilder.html )

So this seems reasonable - my branch looks for a single constructor, but if you had multiple constructors, it could simply satisfy the one with the correct available parameters.

• several methods in the factory with the same return type, being able to map them to different sub-types (the “More configuration options”, with fast-car → Porsche & clean-car → Prius, from Guice's javadoc: http://google-guice.googlecode.com/svn/trunk/javadoc/com/google/inject/assistedinject/FactoryModuleBuilder.html )

I think this can be done.

• should the assisted parameters be injectable to fields (and methods when/if we support them) or only as constructor arguments?

I like the option of both. My branch does both, but at some cost of "mental api load", though I think if we disambiguate cleanly without too much hassle, it's not that much extra mental burden. And it lets people choose how to structure their code.

Once we settled on the use-cases:

• how do we want to disambiguate arguments of the same type?

I put this in my doc, but I'll put it here

• Let @Param objects take labels to associate them in any order?
  • e.g. Blah(@Param(“foo”) String foo)
  • Provides most flexibility
  • Must be done on both value type and factory interface to make a match.
  • More verbose and duplicates information.
• Let @Param objects take numerals to associate them to the factory interface in method argument order?
  • Blah(@Param(2) String foo, @Param(1) String bar)
  • Terser and doesn’t add much space.
  • No duplication of information - @Param(numeral) need only be used on the value-type
• Both can be backed up with extra checks in the compile time analyser to catch typo error.
• Either solution could be made optional and only used for disambiguation where such is necessary. This is similar to how Guice doe sit today.
• If we went with constructor-only, then no disambiguation would be needed, nor ordinals, because constructors and methods have deterministic ordering of parameters, and injectables could be subtracted and the rest simply played out in order (my initial version).

My preference is @Param(numeral) (or @Arg - it's shorter), or labels but only requiring @Arg(label) if there is ambiguity.

Christian.

[Thomas Broyer]

On Monday, January 28, 2013 8:11:34 PM UTC+1, Thomas Broyer wrote:

Back to basics, which use cases do we want to support? and to which extent? (i.e. would there be easy workaround?)

• binding the factory to a sub-type of the declared return type (the Payment→RealPayment from Guice's wiki: https://code.google.com/p/google-guice/wiki/AssistedInject)
• a factory interface from a 3rd-party library (e.g. java.util.concurrent.ThreadFactory)
• a class that is injectable but, for some use case, we want to provide some dependencies through factory arguments (the class could possibly be injected directly elsewhere in the app)
• several methods in the factory with the same return type, dispatching to the appropriate constructor/members of the created class (the “Multiple factory methods for the same type” case from GUice's javadoc: http://google-guice.googlecode.com/svn/trunk/javadoc/com/google/inject/assistedinject/FactoryModuleBuilder.html)
• several methods in the factory with the same return type, being able to map them to different sub-types (the “More configuration options”, with fast-car → Porsche & clean-car → Prius, from Guice's javadoc: http://google-guice.googlecode.com/svn/trunk/javadoc/com/google/inject/assistedinject/FactoryModuleBuilder.html)
• should the assisted parameters be injectable to fields (and methods when/if we support them) or only as constructor arguments?

Once we settled on the use-cases:

• how do we want to disambiguate arguments of the same type?

Here's a round-up of the proposals wrt if/how they address the above use-cases: https://docs.google.com/spreadsheet/ccc?key=0Agcd-Zsy2T-YdFRrSS0xMUduSmdUV2ZVOFBObTF6TEE

Everyone from the group can edit, so feel free to complete or correct the information.

[Christian Gruber]

So, my intention was to evolve my own code to fit the needs of what was
described here - either before it was accepted, or in iterations,
getting the first use-cases in place, so long as further use-cases
weren't API-incompatible. So looking at the spreadsheet, it's unclear
how to proceed. I think the main thing that would shape the direction
of my own code would be deciding on how we'll disambiguate parameters
and class "ingredients", and how we want to handle cases where we need
to specify against code we don't control (presumably in a @Module add-on
capability or some such, since Modules are the configuration point of
access.)

So I'm not sure there's really a huge bake-off. Once the use-cases we
want to support in Dagger are chosen, I'm sure Yuri's or my or Jesse's
code could be written or altered to take into account any of it.

So, The real question isn't whether our proposals satisfy these - it's
whether Dagger should support these. Whose code we use is sort of a
secondary matter, I expect.

Christian.

On 28 Jan 2013, at 18:07, Thomas Broyer wrote:

> On Monday, January 28, 2013 8:11:34 PM UTC+1, Thomas Broyer wrote:
>>
>> Back to basics, which use cases do we want to support? and to which
>> extent? (i.e. would there be easy workaround?)
>>

>> - binding the factory to a sub-type of the declared return type (the

>> Payment→RealPayment from Guice's wiki:
>> https://code.google.com/p/google-guice/wiki/AssistedInject)

>> - a factory interface from a 3rd-party library (e.g.
>> java.util.concurrent.ThreadFactory)
>> - a class that is injectable but, for some use case, we want to

>> provide some dependencies through factory arguments (the class could
>> possibly be injected directly elsewhere in the app)

>> - several methods in the factory with the same return type,

>> dispatching to the appropriate constructor/members of the created
>> class
>> (the “Multiple factory methods for the same type” case from
>> GUice's
>> javadoc:
>> http://google-guice.googlecode.com/svn/trunk/javadoc/com/google/inject/assistedinject/FactoryModuleBuilder.html
>> )

>> - several methods in the factory with the same return type, being

>> able
>> to map them to different sub-types (the “More configuration
>> options”, with
>> fast-car → Porsche & clean-car → Prius, from Guice's javadoc:
>> http://google-guice.googlecode.com/svn/trunk/javadoc/com/google/inject/assistedinject/FactoryModuleBuilder.html
>> )

>> - should the assisted parameters be injectable to fields (and methods

>> when/if we support them) or only as constructor arguments?
>>
>> Once we settled on the use-cases:
>>

>> - how do we want to disambiguate arguments of the same type?

>>
>> Here's a round-up of the proposals wrt if/how they address the above
> use-cases:
> https://docs.google.com/spreadsheet/ccc?key=0Agcd-Zsy2T-YdFRrSS0xMUduSmdUV2ZVOFBObTF6TEE
> Everyone from the group can edit, so feel free to complete or correct
> the
> information.
>

> --
> You received this message because you are subscribed to the Google
> Groups "Dagger Discuss" group.
> To post to this group, send email to dagger-...@googlegroups.com.
> To unsubscribe from this group, send email to
> dagger-discus...@googlegroups.com.

> For more options, visit https://groups.google.com/groups/opt_out.

[Christian Gruber]

Hey Sam. Do you have any preferences or recommendations in terms of the
end-user API?

Christian.

> To unsubscribe from this group and stop receiving emails from it, send

> an email to dagger-discus...@googlegroups.com.

> To post to this group, send email to dagger-...@googlegroups.com.

[Sam Berlin]

I do, but was waiting to see how the discussion played out first. :-)

... let re-read the thread, and I'll gather my thoughts & reply later tonight or tomorrow afternoon.

 sam

To post to this group, send email to dagger-discuss@googlegroups.com.
To unsubscribe from this group, send email to dagger-discuss+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.

--
You received this message because you are subscribed to the Google Groups "Dagger Discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to dagger-discuss+unsubscribe@googlegroups.com.
To post to this group, send email to dagger-discuss@googlegroups.com.

[Thomas Broyer]

On Tue, Jan 29, 2013 at 3:23 AM, Christian Gruber
<christiane...@gmail.com> wrote:
> So, my intention was to evolve my own code to fit the needs of what was
> described here - either before it was accepted, or in iterations, getting
> the first use-cases in place, so long as further use-cases weren't
> API-incompatible. So looking at the spreadsheet, it's unclear how to
> proceed.

Sorry, I should have been clearer.

My intent was to point any incompatibility between proposals and
proposed use-case, such that if we think we should support a given
use-case, an incompatibility would be a showstopper.
The fact Jesse's proposal makes use of standard injection rules out
selection among overloaded constructors (it actually doesn't rule out
overloaded factory methods though, if we see factory arguments as
possibly overriding global bindings, the module thus being generated
with overrides=true).
Similarly, I doubt Yury's proposal can support the “same return types
mapped to different subtypes” without something to disambiguate
factory methods (following his proposal that everything is configured
in the module, that would probably involve putting the name of the
factory method in the @Factory annotation, but that would prevent
having overloaded methods –same name, same return type– mapped to
different constructed types).

Some use-cases will shape the API (see above) while others will drive
the implementation (with your proposal, injecting fields means
potentially generating "accessor helpers" in other packages to be able
to inject non-public fields of a super-class from another package;
injecting only to constructors makes things much easier; with Jesse's
proposal we might want to use overrides=true on the generated module
to support the “injectable class” and/or “overloaded factory methods”
use-cases, which has consequences on the amount of validations we can
do at compile-time (IIUC, correct me if I'm wrong)).

> So, The real question isn't whether our proposals satisfy these - it's
> whether Dagger should support these. Whose code we use is sort of a
> secondary matter, I expect.

Of course!
But Yury's proposal is rather different from Jesse's or yours wrt the
API (i.e. it's not just about the underlying code), and as we started
talking about implementations already, I thought it'd be interesting
to know which API and implementation can support (more or less easily)
which use-case; to help shape the API and then possibly decide which
overall orientation to use for the implementation.

If there are use-cases you (“you” being Yury, Jesse or yourself) think
Dagger should support, and your proposal can be tweaked to accommodate
them, then you can amend it and update the spreadsheet.

But first, as I suggested in my previous message: let's settle on the
use-cases we want to support. Should we do a poll? (more a “I could
hardly live without this” than a “oh yeah, good idea” kind of poll)

[Christian Gruber]

On 29 Jan 2013, at 5:25, Thomas Broyer wrote:

> On Tue, Jan 29, 2013 at 3:23 AM, Christian Gruber
> <christiane...@gmail.com> wrote:
>> So, my intention was to evolve my own code to fit the needs of what
>> was
>> described here - either before it was accepted, or in iterations,
>> getting
>> the first use-cases in place, so long as further use-cases weren't
>> API-incompatible. So looking at the spreadsheet, it's unclear how to
>> proceed.
>
> Sorry, I should have been clearer.
>
> My intent was to point any incompatibility between proposals and
> proposed use-case, such that if we think we should support a given
> use-case, an incompatibility would be a showstopper.
>

> [snip]
>

that all makes sense.

> But first, as I suggested in my previous message: let's settle on the
> use-cases we want to support. Should we do a poll? (more a “I could
> hardly live without this” than a “oh yeah, good idea” kind of
> poll)

That makes sense also, with one caveat…

I think there are some implications we should flesh out of the different
proposals before the poll. For example - if we need additional
injection helpers to handle cross-package injection issues with my own
proposal, is that a problem? Or is it alternatively a problem to simply
require that a factory can only create things in its own package, or
inject things that are accessible to it? I can probably live with that
limitation in order to make the infrastructure simpler, just like we
live with limitations like no private injection of members. But maybe
it's worth it to have those pieces. That's hard to judge until real
code hits real day to day usage.

Which implications are a show-stopper, and do they outweigh the
simplicity or other choices in the API? Is there some sort of 80/20
rule we can apply - we seem to have done so for other parts of Dagger.
My own design goal is to make features as clean and simple in the API
for the majority use-cases as possible, possibly at the cost of some
more complex use-cases (things that can be worked around with
`@Provides` bindings, for example.

In short, I'm not sure how to do this in a poll. It's not as simple as
X feature is supported by Jesse, Christian, or Yuri's proposal - but the
more I look at it, and considering your example above, HOW does it
support these things matters, I suspect.

Christian.

[Sam Berlin]

So keep in mind I have very little knowledge of Dagger's internals and how it's actually implementing this stuff, so my thoughts here might not have much practical application.

Stepping through by use-case (basically by reading FactoryModuleBuilder's javadoc) --

1) Basic usage.

    Foo {

       @Inject Foo(NormalDep, @Assisted AssistedDep) { .. }

       interface Factory { Foo create(AssistedDep); }

    }

    -- I like the fact that @Assisted exists on the assisted dependencies.  It's in-code documentation that explains which things are likely to change more frequently, and which are more "static" dependencies.  It also forces the reader to recognize, "Oh, I can't just get a Foo directly, I have to do it through a Factory and supply these things."  Sure, that isn't strictly an improvement, but I think the clarity it gives the code is a net win.

 2) Multiple factory methods per type.

     Foo {

        @Inject Foo(NormalDep, @Assisted OneThing) { .. }

        @Inject Foo(NormalDep, @Assisted OrAnother) { ... }

        interface Factory {

          Foo create(OneThing);

          Foo create(OrAnother);

     }

     -- This is a power-user feature, no question about it.  When I've needed it, it's been a life-saver.  Manual factories are error-prone and annoying, so it's nice that AssistedInject supports this case.  I could understand ditching it for Dagger because of it's "small and to the point" lifestyle (get it?!), but I know I'd be frustrated when I wanted it and it didn't exist.  If it doesn't cause complications in the implementation, and it's just as easily traceable through the code-gen, then I don't see a reason to ditch it.  Note that Guice requires the constructors be annotated with @AssistedInject instead of @Inject, but I don't think that's totally necessary to keep.  I'm actually wondering if we should change that in Guice.

 3) Factories for different return types.

     Foo { ... }

     Bar { ... }

     Factory {

       Foo create(..);

       Bar create(..);

     }

     -- This is neat, but I've never needed it in practice.  It's easy to do on your own, creating factory just for Foo, another for Bar, and a composite factory that injects each other the others & delegates to them.  It's easier than doing (2) on your own, because that would otherwise require duplicating all the parameters from Foo into Factory, and making sure you use Provider<T> instead of <T> for them (which is a common mistake in hand-rolled factories).

 4) Factories with more specific return types.

     interface Foo { 

         Factory { Foo create(...); }    

     }

     class SpecialFoo implements Foo { .. }

     -- This isn't terribly common, but when it happens, I'm always happy I can swap out the return type for another subclass very easily.  I want my classes to take Foo.Factory, not SpecialFoo.Factory.  But, it's pretty easy to workaround this too: just have @Provides Foo.Factory provideFF(SpecialFoo.Factory sff) { return sff; }, and have a little boilerplate where SpecialFoo has its own Factory that implements Foo.Factory & returns a SpecialFoo.  So I don't think anything special needs to happen in Dagger to support this case.  If we wanted to support (3) then this might not work as well, because it'd be harder to specialize Factory for both Foo & Bar at the same time... but if we ditch both (3) & (4), I think we're in the clear.

 5)  Distinct parameter types.

      Foo {

         @Inject Foo (NormalDep, @Assisted("start") Date, @Assisted("end") Date) { .. }

         Factory { 

             Foo create(@Assisted("start") Date, @Assisted("end") Date);

         }
      }

      -- This doesn't come up too often, but when it does, it'd be really frustrating if there was no way to solve it short of hand-rolling the factory.  @Named works too, but that means we're losing the in-code documentation of @Assisted.  Basically, this should be solved according to however (1) is solved.

 6) Annotating factory methods.

     Factory {

         @Named("hot") create(..);

         @Named("orNot") create(..);

     }

     -- I've never seen this used, ever. I imagine it's probably useful when it is used, but I have no argument for keeping it. Along the lines of (3) & (4), I'd say this gets ditched too.

In a nutshell:

   a) @Assisted is useful for in-code documentation, I vote to keep.

   b) Hand-rolling factories is horrible.  If dropping a feature means someone has to hand-roll a factory (mirroring all the dependencies as Provider<T>), don't drop the feature.

   c) Ditch everything else.

sam

On Mon, Jan 28, 2013 at 10:28 PM, Christian Gruber <cgr...@google.com> wrote:

To post to this group, send email to dagger-discuss@googlegroups.com.
To unsubscribe from this group, send email to dagger-discuss+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.

--
You received this message because you are subscribed to the Google Groups "Dagger Discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to dagger-discuss+unsubscribe@googlegroups.com.
To post to this group, send email to dagger-discuss@googlegroups.com.

[Thomas Broyer]

That'd violate JSR 330:
http://atinject.googlecode.com/svn/trunk/javadoc/javax/inject/Inject.html
“@Inject can apply to at most one constructor per class.”

We don't strictly follow the JSR already, but we are constraining
what's possible (no private ctor or field, no method injection, class
must have an @Inject field or ctor to be default-injectable) and the
constrained use-cases are still valid per the JSR. We don't
strictly-speaking violate the JSR, we just don't implement all its
features.

> 3) Factories for different return types.
> Foo { ... }
> Bar { ... }
> Factory {
> Foo create(..);
> Bar create(..);
> }
> -- This is neat, but I've never needed it in practice. It's easy to do
> on your own, creating factory just for Foo, another for Bar, and a composite
> factory that injects each other the others & delegates to them. It's easier
> than doing (2) on your own, because that would otherwise require duplicating
> all the parameters from Foo into Factory, and making sure you use
> Provider<T> instead of <T> for them (which is a common mistake in
> hand-rolled factories).

I use this (factories for different return types) a lot with GIN (GWT
apps using Places+Activities, using a single big factory in the
ActivityMapper; maybe you can get in touch with the Google Groups team
as they seem to be using Activities) but I must say this is mostly to
cut down boilerplate: with one factory for 10 types (or more), I only
have one FactoryModuleBuilder to install. The downside of it is that
the factory is not defined close to the constructed type(s) but in
practice it has never be a problem for me (the factory is defined
close to where it's used, and changing the assisted parameters would
mean changing the call points too).
If/as Dagger removes that FactoryModuleBuilder boilerplate then I
could probably leave without it (it would still add some boilerplate
by requiring 10s of "@Inject Xxx.Factory xxxFactory" in the
ActivityMapper rather than one "@Inject BigFactory factory", but it's
no big deal)

+1

Basically, everything with an "easy workaround" in
https://docs.google.com/spreadsheet/ccc?key=0Agcd-Zsy2T-YdFRrSS0xMUduSmdUV2ZVOFBObTF6TEE
can be left out (note: I added the "factory with different return
types" use-case to the spreadsheet; it was a given for me but as Sam
is suggesting ditching it…)
I've never needed a factory for classes that are "otherwise
injectable", and apparently no one else either, so that use-case is
not worth supporting IMO.
My condition for ditching "factories with different return types" is
that we don't require boilerplate code in the module, à la
FactoryModuleBuilder (imagine, for 160 types to construct with a
factory –this is the actual number for our biggest project to date–,
I'd need 160 factories, 160 declarations in modules, and 160 '@Inject
Foo.Factory'; I currently have only 26 factories for all those 160
types). This can be solved by annotating the factory with
@dagger.Factory, as was already proposed.

I'd go even farther as to only support assisted values in constructors
(not fields); a generated factory could then look like:

class Foo$Factory$FactoryImpl implements Foo.Factory {
@Inject Provider<NormalDep> normalDepProvider;
@Inject MemberInjector<Foo> fooMemberInjector;

@Override public Foo create(AssistedDep assistedDep) {
Foo foo = new Foo(normalDepProvider.get(), assistedDep);
fooMemberInjector.injectMembers(foo);
return foo;
}
}

--
Thomas Broyer
/tɔ.ma.bʁwa.je/

[Michael Bailey]

What was the outcome? Will Dagger get assisted inject? :)

[Christian Edward Gruber]

https://github.com/google/auto/tree/master/factory is released in an early (0.1-beta1) form.  Dagger itself won't do assisted inject, as it really is about building factories, and it's appropriate for another project to do that.  Auto-factory does it in a daggery-way (even using similar infrastructure), and produces @Inject-annotated factory implementations which can then be explicitly or automatically included in dagger graphs. 

I should actually do an announcement.

c.

[Christian Edward Gruber]

And do feel free to file feature requests or bugs on the github.com/google/auto project against auto-factory if you have issues, or need other use cases met.  What we have there so far is an initial cut covering some of the use-cases. There's still a ways to go, but I'd like to see real usage needs drive further API design. - Christian.

On Tuesday, 29 January 2013 09:51:27 UTC-8, Christian Edward Gruber wrote:

On 29 Jan 2013, at 5:25, Thomas Broyer wrote:

[Christian Edward Gruber]

Ahh.  I suck at using the groups forum interface. :(  This was meant to be a follow-up to my most recent e-mail.