Teamwork / android-clean-architecture

Showcasing a Clean Architecture approach from our Android applications framework!
https://developer.teamwork.com/
Other
161 stars 16 forks source link

My 2 cents #5

Closed dmarin closed 4 years ago

dmarin commented 4 years ago

As per my discussion with @marcosalis , I would like to put my 2 cents in :)

  1. Some modules should be pure jars. A clear example is Entity
  2. Many third-party libraries are using the api configuration, even when apparently there is no such need. Ie, why entities define as api com.android.support:support-annotations, would not be better to add this as implementation on the modules needing it.
  3. This one is minor, or probably just me going crazy, but I failed to understand why entity is defined as api in all the modules using it except feature1. I understand entity is part of the api and therefore the api configuration makes sense, but why data declares an api dependency to entity if data-access already declares it as api? And what is the rule to skip this dependency in feature?
marcosalis commented 4 years ago

Hey @dmarin , thanks a lot for your feedback! Here's a few comments on your very interesting points 👍

  1. Some modules should be pure jars. A clear example is Entity

Correct indeed. There's no reason for the sample to not do that 👍 We have discussed this internally and the reason we've chosen not to do it in our own projects is that we use Room to annotate business entities and use them as "DB representations" as well. This is a compromise we made in order to avoid having 4 different DTOs for the same entities (response, business entity, DB entity, UI model). This is not ideal since it introduces the need for the Room dependency in the entity layer but, at the same time, it's a change that doesn't propagate outside the boundaries of the layer; from outside the classes themselves, those entity objects appear as plain DTOs (that's an advantage of using annotations: they're not part of the object's public interface, and many frameworks use them for this very reason). We might, in the future, especially since Kotlin has reduced the boilerplate required for this, think of only having an entity interface in the entity layer, and implementing it in the data layer, where then the @Entity and other Room-related details would be added.

  1. Many third-party libraries are using the api configuration, even when apparently there is no such need. Ie, why entities define as api com.android.support:support-annotations, would not be better to add this as implementation on the modules needing it.

We tend to do this for dependencies which are not "layer-bound", especially when we know that those dependencies are going to be used in every module of the app. This simply a way to avoid redeclaration of the same dependency in different places. A typical example is the entity module, since that's gonna be used everywhere from the data layer to the presentation. At the same time, we're a lot more formal when it comes to layer dependencies: that's where using implementation is important.

  1. This one is minor, or probably just me going crazy, but I failed to understand why entity is defined as api in all the modules using it except feature1.

That's just a mistake in the sample (will fix!) 😄 As mentioned above, entity is the only layer which can and should be accessible from all of your layers. If you wanted to be stricter and more dogmatic, you could isolate the entity layer from the view, and create UI models for everything in order to avoid applying business logic where one shouldn't. However, this causes a lot of duplication (sometimes you just don't need a UI model, because it would be identical to your entity) and we didn't find it necessary nor beneficial.

Wish you a Merry Christmas and great holidays! 🎅 🍻

marcosalis commented 4 years ago

PS: you might find this interesting to check out: https://github.com/android/architecture-samples/issues/695

I made a few points about visibility (similar to one of yours) in the "official" Android architecture blueprint Clean sample.

marcosalis commented 4 years ago

Fixed where necessary with #7