ataulm / muvi

Apache License 2.0
39 stars 3 forks source link

Feature/analyse dependencies graph #36

Open condesales opened 4 years ago

condesales commented 4 years ago

This is a proposal on how to handle the dependency between modules. I used Jake Wharton's Gradle script with some minor changes to plot the dependency graph to help visualize it.

brew install graphviz
./gradlew projectDependencyGraph

Original Graph image Proposed Graph image

The changes mainly follow the idea of using dependencies as api instead of implementation wherever I thought it made sense.

The onion diagram from Clean Arch always has the data layer as a superset of domain. By using that idea we would expect that when adding data as a dependency you would have access to its domain I applied the same logic to the cache/remote "data stores" (not sure how you call them)

From the original graph, we could also see that :design_library and :navigation modules are all accessed by the end-feature modules :feed, :actor_details and :film_details. I believe the whole idea of a :core module is to share all the shared dependencies between modules so moved those as api dependencies there.

One interesting case is for the :letterboxd_api. That module theoretically should only be accessed by remote modules but we need it as a dependency to :core because we need to build the dagger graph and provide it as @Singleton. Perhaps by using @Reusable we could achieve the same outcome?

It's important to be intelligent about what goes into :core as dependencies. Only those that are shared across all features should go there as we don't want to provide dependencies that are not needed by features. This could be a big impact on dynamic features.

I'm going to leave some comments on specific lines to explain some specific decisions, but the above summarises the whole idea.

I'd love to hear from you the thought process behind the original graph dependencies setup. I dived into this specifically because I was curious to see how other people have been tackling the same problem, so thanks for sharing!

jamieadkins95 commented 4 years ago

The onion diagram from Clean Arch always has the data layer as a superset of domain. By using that idea we would expect that when adding data as a dependency you would have access to its domain

A nice way of thinking about this is that domain is part of the public API of data, since when you go to data to fetch a list of films, it gives you back some domain objects. You can't use data without also including domain, therefore depending on data should also give you domain.

SAGARSURI commented 4 years ago

@condesales can you share the script or process to generate the DI graph?

condesales commented 4 years ago

@SAGARSURI It's in the PR https://github.com/ataulm/muvi/pull/36/files#diff-731c4a6982ed61c6516c58a09d9456af

SAGARSURI commented 4 years ago

So after looking at the script. I believe I need to update the module names in the rank section? @condesales

SAGARSURI commented 4 years ago

Just curious, When will this be getting merged? @jamieadkins95

condesales commented 4 years ago

So after looking at the script. I believe I need to update the module names in the rank section? @condesales

Yeah, unfortunately, to get a good plot we had to hardcode some of the prefixes/suffixes of the names of the modules.