We decided some time ago in the Android team here at babylon to follow clean architecture. So the code cleaning task began! With a goal to create all new features, and refactor existing ones, using clean architecture. We started with event tracking because it’s super important for the business; as you can imagine we have loads of different events being tracked throughout the babylon app.
Commence Code Cleaning
When we started, our setup looked like this:
Abstract tracking event class with the required methods to get event name, properties and others.
One sub-class for each event type (some of them handling different states)
Using an event bus to trigger every event and a generic event subscriber reading the event properties and calling the API to track the events.
3 constants God classes for event names, properties and values with several constants shared between events
This was working fine, but it wasn’t looking great. It violated a few clean architecture principles, so it didn’t make the cut.
Tackling the problem part 1
After adopting clean architecture in the project, creating a tracking interactor (use case/command) and a repository to handle the tracking had to be mandatory so this is where we started:
You might have noticed that we were using a TrackingEventData class. This was the new Event class we created using AutoValue and a fancy builder pattern, to replace the TrackingEvent abstract class mentioned previously. With this change we no longer needed an event class for each event/event type.
We still wanted to get rid of the constants God classes and at this stage we had no place to keep them. As we wanted to keep the constants somewhere specific to each event, we decided to create a mapper class to map each event into the new TrackingEventData model.
At this stage at least we could control all the constants frenzy going around as each event mapper would contain the constants specific for that event and those wouldn’t be shared or used somewhere else apart from the mapper test class.
This was looking nice, now all our presenters and interactors could just inject the event mapper and use the tracking interactor or use the gateway respectively. Tracking events was now as easy and simple as this:
So all new events were being tracked like this and YAY we were done, we were handling tracking in a clean way. Or were we? The excitement didn’t last long, when more complex events started to show up and we tried to migrate some of the old ones we found a few problems:
These new mapper classes were becoming kind of a copy of the original TrackingEvents we deprecated with the only advantage being the constants being contained per event
We had to inject the mappers wherever we needed to track the respective event
For simple events with just name we stopped creating mappers because of the above and started using just the builder straight away in the presenters/interactors. Consistency down the pipe.
And how could we handle events with different states and sources?
So back to the drawing board as we hadn’t found ‘The Solution’ yet.
Tackling the problem part 2
Ok, so if the mapper classes were almost a copy of the original TrackingEvent models and with the disadvantage of having to inject them everywhere why did we need them in the first place? We realised the mistake and brought back the original abstract class and child event classes as we realised the advantages were superior to any disadvantages:
The TrackingEvent class already had all the properties we needed to support every type of event
It’s an abstract class so we only needed to implement the required methods for each event. With the fancy AutoValue one we had, most of the properties were set as nullable otherwise every property would have to be set on the builder.
Migrating older events would be much easier as they’re already extending this class. Apart from moving the scattered constants to the right place, replacing the event bus with our interactor would do the job.
So this solved 3 out of the 4 problems, we still had to find the best way to handle different states in events. We wanted to remove as much event information as possible from presenters so we decided to break down events whenever possible and have different events for different states.
For example the VerificationActionEvent we’ve seen above was split into VerificationActionPassedEvent and VerificationActionFailedEvent. This might look a bit overkill but in reality the events are completely independent and decoupled. The constants are duplicated, yes, but they are contained and only refer to the event where they’re declared. It might not make a difference now but if in the future the business wants to track some user properties with the passed event, or a reason for the failed event, we don’t need to touch the other event.
This event breakdown approach is the only rule we’re not applying 100% because for some events the amount of properties (namely user properties in login and registration events for example) is really big and it would be hard to keep track and error prone if some of them would change in the future.
For those cases we decided to keep the respective state/source inside the event as an enum. This way, presenters/interactors only need to pass the enum value and don’t need to be aware of anything else related with the event.
So the TrackEventInteractor and the TrackingGateway stay exactly the same just replacing the TrackingEventData created in part 1 with the original TrackingEvent:
And we don’t have the mapper anymore when tracking and this is how we pass the state/source from the CodeAddedEvent seen above:
And that’s it. That’s how we’re handling event tracking in a clean way. It was a very interesting and fun task for me, particularly as I had no experience in clean architecture prior to joining babylon around 4 months ago.
We went from something that was working fine but we didn’t like, to something we believed was amazing that we soon realised wasn’t, and finally to something we believe is the right way to do it.
Will we feel the same in a few weeks or months? We’re not sure, but if we change our mind, it will only mean we found an even better way to do it. So if it comes to that, we’ll welcome that change. Feel free to ask questions and share ideas in the comments below to let us know what you think. :)
If you liked the article please recommend and follow, and if you see yourself working with an Android team like ours, we’re happy to tell you we’re hiring at the moment. Feel free to check our Android Engineer job spec.