Allow motif scopes to be abstract classes, to allow for internal visibility modifiers#273
Allow motif scopes to be abstract classes, to allow for internal visibility modifiers#273wynneplaga wants to merge 1 commit intouber:mainfrom
Conversation
wynneplaga
force-pushed
the
wynne.plaga/allow_abstract_classes
branch
4 times, most recently
from
bcf306c to
041b836
Compare
wynneplaga
force-pushed
the
wynne.plaga/allow_abstract_classes
branch
from
041b836 to
a880488
Compare
|
|
||
| @Scope(useNullFieldInitialization = true) | ||
| public interface PhotoGridScope { | ||
| public abstract class PhotoGridScope { |
There was a problem hiding this comment.
If we examine any other Dependency injection system, like Dagger component, we would notice that it is also an interface, rather than a class. The underlying rationale, if I have to guess, is because that a DI system is suppose to be functioning like a container that gives us something to use through the access method like public abstract PhotoGridView view();. And that is its only purpose.
If we make it as class, it signals that this class is serving other purpose, which complicates things, especially due to the fact that the DI system is already complicated enough.
Hence, though I am not the original author , I would bet that it is a purposeful decision on making it as interface. And I would still recommend that we keep it as interface. If you have legitimate use case that need to make it as class, we could sync offline in Uber internally to see whether there is any alternative that I could suggest to meet your needs.
There was a problem hiding this comment.
Dagger actually does permit abstract classes to be components (source), as does Metro's graph extensions (source).
I definitely agree that allowing scopes to be abstract classes is less than ideal, but I think preventing modules from encapsulating the types they provide through the DI framework is worse in terms of code cleanliness. I also think it would be trivial to design a detekt plugin that forbids fields in motif scopes.
2 participants
Currently Motif only allows for interface scopes. This poses issues for inter-module encapsulation since all methods have to be public. This diff allows Motif scopes to be abstract classes as well, allowing modules to keep dependencies internal if they wish.