android10 / Android-CleanArchitecture

This is a sample app that is part of a series of blog posts I have written about how to architect an android application using Uncle Bob's clean architecture approach.
Apache License 2.0
15.51k stars 3.32k forks source link

Refactor project structure to one single module? #112

Open android10 opened 8 years ago

android10 commented 8 years ago

Hi guys, as you can see I have been working on a single module project structure. Here is the PR: https://github.com/android10/Android-CleanArchitecture/pull/103

I'm not saying that I will land it but I do want to start a discussion around it and see pros and cons for either a single module project or multi module project.

The first couple of things that came to my mind:

Advantages:

A few extra eyes are very appreciated, so let's try to find the best way to keep on improving this codebase.

@spirosoik @Trikke @caseykulm I mention you guys since have been collaborating a lot. Thanks! Feel free to involve anyone here :smiley:

Trikke commented 8 years ago

Thanks for the mention, @android10! I'll just give my two cents here and bring some points up for discussion so we can hear everyone's ideas about them. I've taken a quick look at the current branch. I'm a fan of "package by feature", but that is usually incrementally more difficult to implement than "package by layer". It can be hard to describe layer bounds and make mistakes against the layered architecture. I already like the work you've done on that branch, and here are a few ideas/remarks.

That core package I've never liked packages that very broadly named, "core", "util", "ui", "helper", ... So i'm wondering why that "core" package is needed? Because anything under the "view" package could also be classified as part of the "core" of the app, so should that also go under "core.view"? I'm in favour of dumping the "core" package. The "di" package is as much of a feature in the app as any other. If a coworker wants to switch to a different DI framework, they should easily find that feature.

Package by feature, then by layer Another idea is to first package by feature, then package by layer. For a simple sample app as this, the "user" feature is already quite full of classes. What if you'd even further split up each feature by layer? This would make sense if you have large feature teams working on your app and have guys responsible for UI, others are experienced in data access and so on. I've quickly drafted a treeview to give you an idea.

app
└───user-feature
    ├───data
    │   │   UserRepository
    │   │   UserDataRepository
    │   │   ...
    │
    ├───domain
    │   │   GetUserDetails
    │   │   GetUserList
    │   │   ...
    │
    ├───view
    │   │   UserListActivity
    │   │   UserListPresenter
    │   │   UserListAdapter
    │   │   ...
    │

The other feature packages make sense. A package for the "navigation" feature. A package for the "data retrieval" feature, and so on.

These are just a few ideas and are mainly meant to fire up the discussion. I'd be happy to hear anyone's thoughts. If i have so more time, i'll go over the branch in more detail.

real-malan commented 8 years ago

Hey,

think about using veripacks in this approach so application flow would go as intendent (view -> domain -> data).

spirosoik commented 8 years ago

Hi guys. thanks for mentioning @android10!

I will go over the advantages to describe my thoughts as I am as supporter of the 3 modules/layer approach.

Scalability: I think that this depends on how the team is structured. For example for a very big organisation there are teams for specific layers/modules in every kind software (mobile/web etc.) . I think that the package by feature makes sense on presentation and domain layer where it's much easier to apply it and maintain it. The data and the core layer must be abstract, independent and must expose only the necessary things which will be used by the upper layers. The data and the core layer must be able to run their tests without any dependency in the domain and the presentation layer. For sure in a 3 modules/layer approach it can be a 4th core module. This is the approach which we are using here also we have a separated core module but not for DI or views. We use it for some tools which we want to be available in every layer.

Scope: I think this can be done easily with current implementation also but it needs changes using the DI. I will refer again the Effective Java book and especially the Item 13 Minimize the accessibility of classes and members. This can be done with Dagger2 according to different modules, components and scopes. That's why the Dagger2 give us power.

Build System: I think this is not valid as there are plenty of optimisations and techniques which we can apply and boost the performance of the build process.

Now for the Drawbacks section I want to note the Dependency with the framework which is very very important. Many Devs use context anywhere they need without think about, "is it really needed here?" and then the Business logic is coupled with the Android framework. So if we have a clear independent java module, for me raises automatically the rule do not couple the business logic with the framework. This will help the team for code reviews also. Imagine that a newcomer (even a mature member) starts writing code and the code reviewer realises that the whole business logic or part of it, is coupled with the framework (I know that we setup rules by the first day but it's possible to have such kind of mistakes).

Lastly which you didn't mention guys and I think it's very important is that with this kind of approach we aren't able to run the different layer tests independently in a CI environment and in the development process. With the one module approach it's obvious that I must run every test which it's not efficient. Yes I know you can do it via Android studio (for dev process) but there are different scenarios when I am using a CI and I want to have different kind of build processes.

That's all guys, these are my comments :) . So food for thoughts.

This kind of discussion would be great to be done in a pub with :beers:

Rainer-Lang commented 8 years ago

I like to have a "feature-structure"... “Package by features, not layers” @cesarmcferreira https://medium.com/the-engineering-team/package-by-features-not-layers-2d076df1964d

Rainer-Lang commented 8 years ago

Look at point 10 and especially 15 ;-)

https://medium.com/@cesarmcferreira/building-android-apps-30-things-that-experience-made-me-learn-the-hard-way-313680430bf9

Trikke commented 8 years ago

Hi guys, I had a small brain fart about this. I think @spirosoik makes some valid points, and most people are sold on the current module setup of this project. It's mentally easier to organise and everything that @spirosoik said about that setup makes sense. I'd like to throw around the idea keeping the "layered module" setup(or even refining it more) and then packaging everything by feature. Currently this is not fully the case. If you would/could view it from a high level in some Tool Window in your favourite editor and merge packages from all modules, you'd essentially see everything packaged by feature.

If you look a the current structure, you come across packages as "repository", "interactor", "presenter", ... which indeed contain all related classes. The "repository" packages contains the "UserRepository", the "PhotoRepository", the "WhateverRepository". I think we should split these up by feature. Here's a package tree to clarify it more.

data
└───repository
    │   DefaultRepository
└───cache
    │   ACacheImplementation
└───user-feature
    │   UserDataRepository
    │   UserSqlCache
    │   ...
└───photo-feature
    │   PhotoDataRepository
    │   PhotoFileCache
    │   ...
    │
domain
└───interactor
    │   ObservableInteractor
└───user-feature
    │   GetUserListInteractor
    │   GetUserDetailsInteractor
    │   UserRepository
    │   ...
└───photo-feature
    │   GetPhotosFromUserInteractor
    │   PhotoRepository
    │   ...
    │
presentation
└───navigation
    │   Navigator
└───view
    │   BaseActivity
└───user-feature
    │   UserListPresenter
    │   UserListActivity
    │   UserListAdapter
    │   ...
└───photo-feature
    │   PhotoBrowsePresenter
    │   PhotoBrowseActivity
    │   ...
    │

This way, we keep certain features (navigation, caching,...) in their current layer and have the features of what our app actually "does" grouped in their respective packages.

Rainer-Lang commented 8 years ago

@Trikke I like your "fart" ;-) I find your suggestion will help to keep a good overview over the features - but keeps the 3 module structure. I see the big advantages to have all separated in modules. This makes it easier to avoid mistakes.

android10 commented 8 years ago

Nice discussion guys. I see there are mixed feelings around this PR and honestly I don't see any solution better than the other. One thing to clarify: when talking about features, I mean user facing features, so the rest of the packages don't count here: for example core and di are used to organize and group infrastructure stuff, things that are part of the app and used across it. Cache is not a feature nor util or helper.

android10 commented 8 years ago

I might not merge this PR for now but I will link this to the articles so people can participate and leave their 2 cents on this approach.

Trikke commented 8 years ago

@android10 in another issue you talked about working on projects at Soundcloud where projects exists within a single module. Is there a difference in setup between those and this PR? Or is the 3-tier module setup something you created because you liked it more, reflecting on your work at Soundcloud?

android10 commented 8 years ago

The 3 module project was an initiative to completely run away from android, specially on the domain layer which is a pure java project favoring unit tests, since a couple of years ago, it was very difficult to setup robolectric and junit so you had to create a separate module to test another module. (that does not happen anymore fortunately, buy you can check the history of this repo when I got rid of it).

As I put in the description, I think 3 modules bring a little bit of overhead and that is the reason why I wanted to start this discussion (plus the other disadvantages I mentioned in the description). I'm not saying our solution at SoundCloud is perfect but wanted some input and thoughts coming from the community). Also we are not using a pure Clean Architecture approach, although is similar.

If you are curious about how we work and have a taste of how our app works here are 2 interesting videos:

https://www.youtube.com/watch?v=R16OHcZJTno https://www.youtube.com/watch?v=uHy_54Dpcj0

Trikke commented 8 years ago

Interesting videos to watch, thanks! The first guy talks about a "featurized layers", which i think is something i suggested here as well.

I'll look at the second video once i've got some more time.

lenguyenthanh commented 8 years ago

I agree with @android10, the 3 modules make us hard to manage. It is quite over engineered. Testing is much more easier now. We can keep dependency rule and layers boundary by training and code review.

Rainer-Lang commented 8 years ago

@android10 You wrote that you won't merge this PR. Will be this "all-in-one"-branch also updated in future - or is this effort you made "lost"?

android10 commented 8 years ago

@Rainer-Lang for now I will keep it open and try to maintain it, since there are mixed feelings and I'm not 100% sure yet. Any idea/feedback/input on how to keep both approaches?

spirosoik commented 8 years ago

@android10 very nice presentation by Matthias.

Rainer-Lang commented 8 years ago

@android10 Sorry for the delay...life is sometimes hard.... :( Thanks for letting it open and keep the discussion.

For keeping the both synced - I'm sure this could only be done by hard work - every pr/change will have to be done in both. PUH I don't see another chance... maybe here are some contributors who can help with this "problem".

amondnet commented 8 years ago

How about this?

https://codelabs.developers.google.com/codelabs/android-testing/index.html#4

Notes app structure: Feature separation by packages

Instead of using a package by layer approach, we have structured the application by package per feature. This greatly improves readability and modularizes the app in a way that parts of it can be changed independently from each other. Each key feature of the app is in its own Java package. Let’s take a quick look:

Package: com.example.android.testing.notes

package desc
.addnote “Add note”: Adding a new note
.data Data: Storage of notes - this contains the model layer
.notedetail “Note Detail”: Showing details for a single note
.notes “Notes”: Showing a list of all stored notes
.statistics The statistics screen
.util Utility classes used in various parts of the app, e.g. for handling file access

Each feature package (shown in bold above) contains a Contract interface - this defines the interface for the View and Presenter callbacks of each feature.

This makes it very clear which classes belong to the same feature and how they are interconnected.

https://github.com/googlesamples/android-architecture https://github.com/googlesamples/android-architecture/tree/todo-mvp/todoapp https://github.com/googlesamples/android-architecture/tree/dev-todo-mvp-clean

RowlandOti commented 8 years ago

@Trikke and @android10 , I did try to implement organisation by layer and then by feature. Its incredibly difficult without breaking boundaries. I also integrated Retrofit for the API. Nevertheless, here is a sample project. https://github.com/RowlandOti/Doea

FranRiadigos commented 8 years ago

Hi, I was wondering what could happen with this structure once the Android Instant App get released. I couldn't access the docs yet, due to the limited access for developers, but I can imagine from the latest Google IO that it can be handled by modules.. So we might find difficulties following this structure. What do you thing?

Trikke commented 8 years ago

I don't see what the Instant Apps feature has to do with this Architecture. The first 2 questions of their FAQ are:

Do developers need to build two different Android apps now? Developers only need to maintain one project with one source tree. Developers simply configure the project to create two build artifacts: the installable APK and the instant version. Some developers can take less than a day to get up and running, though the effort involved varies depending on how the app is structured.

What Android APIs and functionality can Instant Apps use? Android Instant Apps functionality complements an existing Android app, but does not replace it. Android Instant Apps uses the same Android APIs, the same project, the same source code. Android Instant Apps restricts some features that might not match users' expectations of an app that is not installed. For example, an Instant App can't use background services, do background notifications, or access unique device identifiers.

FranRiadigos commented 8 years ago

What I mean is, for the instant build version to be done, would we need to create a separate module for that? I didn't see any sample yet, that's why I'm asking. Anyway if I'm not wrong I remember to have heard something about modules in the IO.

Trikke commented 8 years ago

So what you are asking is information none of us have. There's no sample code from Google, no technical guides or anything. It seems like something in a testing phase with the big developers and there's a mailing list you can subscribe to for updates.

You'll have to wait and see how it pans out if they ever release it. Then we'll know how to implement that feature. But going by their FAQ, i imagine it will be just some gradle config and probably a whole lot of checks for features that these Instant Apps can't use.

FranRiadigos commented 8 years ago

I think you could access the testing phase from this link https://docs.google.com/forms/d/1S3MzsMVIlchLCqyNLaFbv64llxWaf90QSeYLeswco90/viewform?fbzx=-652575268842575597

Anyway thank you for the info @Trikke

kevin-barrientos commented 7 years ago

What do you do when you have features that are somehow related to each other but at the same time they should be independent?

For instance suppose that your app should have a module to manage an Inventory of products and another feature to create Orders. In the Inventory feature you can see a list of products and their details while in the Orders feature you are required to enter de Order details and select items. In order to select the Items for an Order the app needs to show a list of products then the product detail and let the user enter the price and quantity.

With the example above, I would like to reuse the Inventory module in the Orders module to display the list of products and their details but how would you recommend to pass the 'selected' product back to the Order module without letting the Inventory module know about the Orders?

So basically the applications paths would be:

kevin-barrientos commented 7 years ago

@android10 I think that the problem that I encounter when packaging by feature is when two features share a Use Case. What should we do then? Do we use the one that is already written in Feature A or do we rewrite it in Feature B? Neither feels good. If we reference a UseCase from another package there is no garantee that someone would change it and break the other feature. But if we rewrite it we end up with duplicated code :s

artworkad commented 7 years ago

@kevin-barrientos if their are closely related it probably makes sense merge them into one feature. Or you could create a parent feature that contains a core, feature-a and feature-b package.

Thansoft105 commented 7 years ago

nice talk...first time am here...thanks all for you guidance ...

DenisBronx commented 5 years ago

Hi guys, I just wanted to give my 2 cents on this. I've been looking at the packaging options from a while and I see that developers implement or know only about package by feature and package by layer. Now as everyone mentioned before package by layer is very easy and natural to achieve as the only dependency issue is between layers while package by feature is harder as there is not always a clear logical separation. On the other end package by feature is less coupled and more cohesive since when you need to create/modify some implementation the number of modules you are compiling/releasing is smaller then with the first approach (every time you add a new feature all the modules needs to be recompiled and released in the package by layer approach).

Now we see that package by feature is clean and cohesive but hard to achieve without workarounds (like rewriting code just to have 2 modules independent) while package by layer is coupled but easy to implement and straight forward when you use a layered architecture.

So which one to choose? None of them, they both are or coupled or kill reusability. In Uncle Bob's Clean Architecture book The Missing Chapter written by Simon Brown he shows the package by component option. Package by component is basically a mixture between package by layer and package by feature. It is composed by 2 modules: a feature module that contains the domain and the data layer of a feature and another module for the presentation part.

With this approach there is no need to duplicate business or data logic only because in different screens there is common logic and when a screen needs to show a set of features you just need to import the module (domain+data) of each feature you need.

I might be completely wrong but I think this is a better approach. You can read more about package by component here

android10 commented 5 years ago

Hey @DenisBronx that is very interesting. Thanks for sharing. I will definitely check the package by component approach which seems to help out solving the boundaries problems with package by feature

DenisBronx commented 5 years ago

I also have some additional observations:

-The domain+data block is not an actual module but a "component", in my approach there would be a feature-domain-module and a feature-data-module, these would be grouped by an "abstract module" which will act as the component (basically a glue). -The presentation module would import 1 or more feature components. -There are still some drawbacks of the package by feature like those common classes you end up putting in the core. To be honest they are not a big problem and I would just create coreData, coreDomain, corePresentation... modules for reusing layer specific code. Anyway you would have the same problem with package by layer, just think about the mapper interfaces you need for mapping between layers (I use a simple mapper interface with 2 generic types), you can copy paste it in every layer module or just extract it in a mapper module and reuse it everywhere.