dagger: File name too long when nesting subcomponents

I’m going through the final steps to fully convert our app to Dagger 2, tying the hierarchy of subcomponents together. We have a deep hierarchy of sub components. This means the subcomponent names are too long:

error while writing DaggerDevRegisterAppComponent.DevLoggedInComponentImpl.DevRootActivityComponentImpl.SellerFlow_MobileComponentImpl.HomeScreen_MobileComponentImpl.TenderPath_ComponentImpl.AbstractGiftCardBalancePath_ComponentImpl.GiftCardBalanceInputScreen_ComponentImpl: 
./DaggerDevRegisterAppComponent$DevLoggedInComponentImpl$DevRootActivityComponentImpl$SellerFlow_MobileComponentImpl$HomeScreen_MobileComponentImpl$TenderPath_ComponentImpl$AbstractGiftCardBalancePath_ComponentImpl$GiftCardBalanceInputScreen_ComponentImpl.class (File name too long)

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Comments: 17

Commits related to this issue

Most upvoted comments

Why are you making fun of me? )c:

On Wednesday, August 3, 2016, Gregory Kick notifications@github.com wrote:

@valeriyo https://github.com/valeriyo, I’d rather not add an API for tweaking something that is entirely an implementation detail.

@loganj https://github.com/loganj, if by “flag naming scheme” you mean using top-level classes rather than nested classes, that would be an option, but means that we’d have to generate components very differently. We would either have to make a lot of fields accessible or pass around a bunch of state (probably more than the max number of cxtor args).

@ronshapiro https://github.com/ronshapiro, that’s a variant of flattening the component hierarchy that might dodge some accessibility issues, but does complicate the generated code because you can no longer use the implicit this pointer. We’d have to start keeping track of some number of parent references and traverse the correct number of them to get to some Provider. E.g. this.parent.parent.parent.applicationProvider. Ick. It’d work but it seems like it’d be a little gross.

OK, time to make fun of @py https://github.com/py. Not really, but that is exactly the type of thing we don’t want to show up in a stack trace. We might as well call them $$$FastClassByGuice$$. 😛

@ronshapiro https://github.com/ronshapiro, I’d stick with AtomicInterger b/c incrementAndGet() is useful even if you don’t care about concurrency.

So, here’s what I’m thinking we can do…

  • Let’s use 255 characters as the heuristic. It’s not exactly right, but should cover most cases correctly.
  • Collapse names w/ some version of initialisms, but only when we have to. I.e. walk the whole tree of subcomponents, figure out the name size and just shorten the ones that are required to make them and all of their children fit.

Sound like a reasonable enough plan?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/dagger/issues/421#issuecomment-237394827, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOB0hcM1_U-t6o-t2bo3F4RiBIza_Oiks5qcRXzgaJpZM4Ja9NX .

Sorry, @py. Let me invite you to join in making fun of @pyricau.

Just saw this. I don’t have any bandwidth at the moment. I’ll pass the word around, maybe someone in my team will get pissed and just do it.

A few things here…

  • I think the current naming scheme is pretty ideal for keeping things clear and straightforward. I can’t think of any change that we’d want to just apply across the board that would be shorter while being clearer or even as clear.
  • We could use an alternate scheme for whenever we think the names are getting too long. Unfortunately, “too long” can be hard to predict: https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits
  • Adding javadoc has long been on our list of things to do and if we do change the names, at least javadoc @links back to the component would soften the blow.

A quick strawman for something that might help would be to use initialisms to try to shorten some of these names. Here’s an example if we just did it for any enclosing classes: DaggerDevRegisterAppComponent.DevLoggedInComponentImpl.DevRootActivityComponentImpl.SF_MobileComponentImpl.HS_MobileComponentImpl.TP_ComponentImpl.AGCBP_ComponentImpl.GCBIS_ComponentImpl

This was not mentioned properly in the Github release notes - https://github.com/google/dagger/releases/tag/dagger-2.11-rc2