Some time has passed since the last clean architecture post. During this time the described architecture implementation has shown some of its drawbacks. In this post I will shortly describe the problems with the current implementation and propose some solutions to these problems. As usual you can find the updated version of the demo application Carbrands in its repository at GitHub - Repository Link.
So what are the problems?
All the problems with the proposed implementation actually originate from a single flawed architectural decision - storing scoped object graphs as field in their corresponding Activity. As a result, each Dagger scoped object graph gets destroyed and recreated together with the Activity each time a UI configuration change occurs. This leads to the folowing drawbacks of the architecture:
- Potentially a lot of object instantiations due to creating a new scoped graph and all its objects on each configuration change
- Presenter and Interactor implementations have to rely on the framework’s classes like Bundle and its lifecycle event callbacks in order to save and restore their state.
- Having a Presenter and Interactor tightly coupled to the UI lifecycle makes them very hard to test due to tight dependencies to framework classes
- Long-running operations which need to be atomic cannot be implemented at all inside these modules due to their instability
The following paragraphs provide some more detail for each of the problems above.
Too many object allocations
Well, that’s probably obvious. Recreating Presenter and Interactor instances and all their dependencies on each configuration change is not cheap. Allocating them takes some time and might have a negative effect on the performance of the app. In addition, the extra memory allocations mean more work for the garbage collector, which also leads to sloppy UI performance.
Presenters, Interactors and Bundles. Oh my!
It might not look like a bad idea at first, but it proved to be really painful in the long term. You see, having to save and restore the state of Presenter and Interactor implementations in Bundle increases their complexity with boilerplate code. The initial idea of the clean architecture is to be, well, clean. So pushing bundles and lifecycle methods as deep as an Interactor is contributing to neither cleaner nor simpler code.
In addition, it can also prove to be error-prone. Since Bundle works with key-value pairs, what happens if two components want to use the same key for different things to store? For example this might lead to an Interactor overwriting value related to the state of a Presenter or some value related to the UI, thus producing errors or even crashes due to wrong types. In order to prevent this, one has to come up with a convention that guarantees unique Bundle keys. This further increases the complexity and introduces even more boilerplate code.
Testing, framework classes and lifecycle
Saving and restoring state of Presenter and Interactor through Bundle and lifecycle callback methods makes testing and especially Unit testing very hard. Having references to framework classes instead of only pure Java inside these components means that you have to use libraries like Roboelectric in order to be able to run Unit tests without an emulator. Don’t get me wrong, Roboelectric is great, but in this case it solves a problem, which would not exist at all in a better architecture.
But let’s say the only framework class we have is Bundle. We can mock it easily with something like Mockito, right? The class itself yes, but which values should it return? Which are relevant to the tested component? It means that you have to go through the code of the whole component and check what keys and values it expects to get from the Bundle. In other words, such components’ dependencies are not clearly visible from their public API. This makes writing good unit tests hard, because the test has to know too much.
Long-running atomic operations
Let’s say that an Interactor needs to perform a network request, which does something important like performing a payment. If a configuration change happens while the request is running, we end up with a brand new Interactor which only knows that a request was sent. But it has no idea if it has reached the server, if it has failed or succeeded. It will also never get back the result of this operation coming from the server.
This problem was already discussed into the previous post, which suggests that such Interactor should live in the Application object graph and not in any of the scoped object graphs. This, however, means that once allocated this Interactor will stay in memory as long as the process lives, which is not that efficient. In addition, scoped graph Interactors have to somehow communicate with this special Interactor, which introduces yet more complexity.
How can we do better?
Before we start looking in some aspects of the improved implementation, here’s a link to the actual repository version this post refers to: Repository Link for v2.0.
In order to fix the problems described above we need to tackle the main flaw of the current architecture, namely the fact that the instances of the scoped object graphs are stored inside the Activity. So how can we eliminate this?
The ObjectGraphHolder
It’s actually pretty simple - we just need to move them to a component which lives longer than the UI. This component is the ObjectGraphHolder - a singleton which has a map containing all scoped object graphs as well as the root object graph. It provides a couple of helpful methods which allow for putting, getting and removing ObjectGraph instances from it. It also contains a helper method for creating a scoped object graph by adding modules to an existing one.
private Map<Long, ObjectGraph> objectGraphMap;
private long objectGraphIdAutoIncrement;
public @Nullable ObjectGraph getObjectGraph(long objectGraphId) {
return objectGraphMap.get(objectGraphId);
}
public long putObjectGraph(@NonNull ObjectGraph objectGraph) {
objectGraphMap.put(objectGraphIdAutoIncrement, objectGraph);
return objectGraphIdAutoIncrement++;
}
public void removeObjectGraph(long objectGraphId) {
objectGraphMap.remove(objectGraphId);
}
public static @NonNull ObjectGraph createScopedGraph(@NonNull ObjectGraph parentObjectGraph,
@Nullable Object... modules) {
if (modules != null && modules.length > 0) {
return parentObjectGraph.plus(modules);
}
return parentObjectGraph;
}This implementation maps instances of the ObjectGraph to unique auto-incrementing ids. You might wonder, why not simply use the the Activity.class as a key? You definitely can, but this turns out to be a bad idea, when multiple instances of the same Activity can appear in one or multiple tasks.
Please notice the removeObjectGraph(id) method, which is used to remove object graphs related to an Activity that is finished. In the previous implementation the scoped ObjectGraph was garbage collected each time an Activity was destroyed, but now we have to take care of this ourselves and call this remove method when an Activity is finishing and will not come back.
The new BaseDaggerActivity
The second important component we need to look at is the BaseDaggerActivity. It does not contain instance of its scoped ObjectGraph anymore, but calls the methods of the ObjectGraphHolder. The most important functionality can be found in the snippet below
private long objectGraphId = -1;
private static final String STATE_EXTRA_OBJECT_GRAPH_ID = "objectgraph_id";
@Override
protected void onCreate(@Nullable Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
// Check for stored id and restore it
if (savedInstanceState != null) {
objectGraphId = savedInstanceState.getLong(STATE_EXTRA_OBJECT_GRAPH_ID, -1);
}
// Check if an object graph already exists for this activity
ObjectGraph activityObjectGraph = ObjectGraphsHolder.getInstance().getObjectGraph(objectGraphId);
if (activityObjectGraph == null) {
// Create a new object graph, which adds to the root application object graph
ObjectGraph rootObjectGraph = ObjectGraphsHolder.getInstance().getApplicationObjectGraph();
activityObjectGraph = ObjectGraphsHolder.createScopedGraph(rootObjectGraph, getActivityModules());
objectGraphId = ObjectGraphsHolder.getInstance().putObjectGraph(activityObjectGraph);
}
// Inject dependencies
activityObjectGraph.inject(this);
....
}
@Override
protected void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState);
// Store the current object graph id to survive configuration change
outState.putLong(STATE_EXTRA_OBJECT_GRAPH_ID, objectGraphId);
}
@Override
protected void onDestroy() {
super.onDestroy();
// Any other call to this method is due to configuration change or low memory.
// We want to release the stored object graph only when the activity is truly
// finishing.
if (isFinishing()) {
onDestroyObjectGraph();
}
}
protected void onDestroyObjectGraph() {
// Remove the current object graph from the holder
ObjectGraphsHolder.getInstance().removeObjectGraph(objectGraphId);
}Each time the OnCreate() method is invoked, we check if the ObjectGraphHolder already contains an ObjectGraph with the particular id related to this Activity. If it doesn’t, we create a new scoped graph and put it in the ObjectGraphHolder, which gives us in return a new id for it.
The id, which the ObjectGraphHolder gives us when we put a new ObjectGraph in it, is our only way of accessing it. Therefore, we need to save it through configuration changes. When a configuration change occurs, we first try to restore it and after that we are be able to request the stored ObjectGraph again in the OnCreate() method.
The final piece of the ObjectGraph handling is making sure to remove it from the ObjectGraphHolder when its related Activity finishes. This is performed in the onDestroyObjectGraph() method which is called from the onDestroy() method only if the Activity is finishing.
The new Navigator
The main purpose of the Navigator class is to provide an abstraction around the navigation methods of an Activity such as startActivity() as well as the building of Intent objects. In order to do that, it needs to have an reference to the currently visible Activity. This becomes problematic when the instance of the Navigator is retained through configuration changes, because the Activity it contains gets destroyed. Therefore its implementation has been updated a bit and now it gets the latest Activity through a setter.
private Activity mActivity;
public void setActivity(Activity activity) {
mActivity = activity;
}This setter is being called in the new BaseDaggerActivity, which makes sure it keeps the instance inside the Navigator always fresh. This way, we also prevent leaking a destroyed Activity, because the reference inside the Navigator points always to the newest instance of the currently visible Activity. The following code snippet of BaseDaggerActivity shows how this is performed.
@Inject Navigator navigator;
@Override
protected void onCreate(@Nullable Bundle savedInstanceState) {
....
// Make sure to update the navigator's current activity
navigator.setActivity(this);
onInjected(savedInstanceState);
}
@Override
protected void onStart() {
super.onStart();
// Make sure to update the navigator's current activity
navigator.setActivity(this);
}
@Override
protected void onResume() {
super.onResume();
// Make sure to update the navigator's current activity
navigator.setActivity(this);
}The new Presenter and Interactor lifecycle
Having the whole scoped object graph survive configuration changes means that we have basically no lifecycle at all. Presenter and Interactor instances simply get instantiated through their constructor and then are garbage collected when the scoped ObjectGraph is removed from the ObjectGraphHolder.
However, references to the Views cannot be injected through the constructor of a Presenter anymore. This is due to the fact that when a cofiguration change occurs, we have a new View instance and we have to update its reference to it inside a Presenter. The following snippets show how this is performed in the CarBrandsActivity and CarBrandsPresenter.
public class CarBrandsPresenter implements CarBrandsPresenterInput, CarBrandsInteractorOutput {
private CarBrandsPresenterOutput mView;
....
@Override
public void setView(@NonNull CarBrandsPresenterOutput view) {
mView = view;
}public class CarBrandsListActivity extends BaseDaggerActivity implements CarBrandsPresenterOutput
....
{
@Inject CarBrandsPresenterInput presenter;
@Override
protected void onInjected(Bundle savedInstanceState) {
super.onInjected(savedInstanceState);
presenter.setView(this);
....
}Finally, sometimes we want to get notified when the whole VIPER module is no longer needed, so that we can for example abort any long-running operations inside an Interactor. We can then override the onDestroyObjectGraph() method in our Activity implementation and notify the Presenter and Interactor accordingly. Doing this allows these components to do any required cleaning of resources and should be always performed, so that no memory leaks can occur.
Summary
The new architecture implementation described in this post have evolved over the last several months. The needed changes have been performed in a couple of steps and have proven to be significant improvement over the existing implementation in terms of code simplicity, testing, flexibility and separation of concerns. It is however far from perfect and can always be improved further.