Features: paginated search; deletable previous searches; details tabs; tablet adjustments
Architecture choices: hexagonal approach; LiveData
; coroutines; popular third-party libraries
Difficulties: some struggles with API 19; FragmentPagerAdatper
; testing
As required, the application features:
- A progress bar appears during the search
- Search is throttled, in order to minimize calls to the MusicBrainz API
- A label with the number of results is displayed
- Results are ordered by pertinence score then by sort name
- Results are paginated, scrolling to the end of the list will load the next page
- Result item displays sort name, disambiguated type and a hypothetical list of relevant tags
- A touch on a result item will open the artist details
- Previous searches are ordered by date (most recent first)
- Previous search item displays sort name, disambiguated type and a delete button
- A touch on the delete button, as well as a swipe, will delete the entry
- A touch on the item will open the artist details
- An header displays the name and a random full-width front cover
- The header will collapse when scrolling to give more space to the elements below
- A discography tab displays a list of all the release groups with the small front cover, the title, and the release date
- The discography is grouped by
primary type
, although there are not many artists that have anything else than albums ("Pink" has some singles) - On tablet device, discography will be displayed as a grid instead of a list, with a dynamic number of items by line (notably more items will be displayed on a line in landscape mode than in portrait mode)
- An about tab displays sections of information such as the gender, the date of birth / foundation, the date of death / dissolution, the country, the legal IPI and ISNI codes, the overall rating, a Wikipedia extract and a "learn more" button that will open a navigator query with the artist name.
- On tablet device, the about tab sections are organized slightly differently in order to use the space more efficiently
I like Martin Folwer & Ralph Johnson vision of what architecture means. It could be summarized in "the important stuff, whatever that is" or "every significant decision made so far".
Here are the important choices that I made during the test.
In an hexagonal architecture -also known as ports & adapters-, the domain (core objects model & business logic) is decoupled from the rest of the system : it depends on nothing and everything else depends on it or revolve around it, including the user interface and infrastructure components such as the database or data provider.
Another way of saying it is that dependencies should always go from the outside of the hexagon to the inside, as we can see in the schema below:
While it is often not obvious how to apply this principle to mobile applications where the domain layer is generally kept very thin, it still a good exercise to help thinking and structure code in a modular, business-centric way.
The domain contains domain objects model of course but also:
- APIs that will be consumed/driven, in our case by a GUI. Schematically, this is everything that resides on the left of the hexagon.
- SPIs that serves to communicate with the oustide world. Schematically, this is everything that resides on the right of the hexagon.
It is that dependency inversion application that keeps the domain decoupled.
Note: UI is not considered "domain material" as business logic shouldn't be related to the way data is displayed.
I'm a big believer in mutli-modules approach. This forces us to think about the system dependencies onward, enforcing good practices regarding decoupling and testing. Not to mention the possible compilation optimizations this provides.
You will find the project is split into 4 modules:
:app
: this is obviously the module containing the entry point of the application;:coredomain
: this contains all the domain objects model as well as the APIs and SPIs interfaces;:musicbrainz
: this contains all MusicBrainz-related objects model and APIs;:sharedfoundation
: this contains some utility classes and functions that have no direct correlation with the project.
Now to correlate the hexagon theory with practice. We can look at how the search artist details feature fits into the project hierarchy:
com.jbr.coredomain.artistdetails
package:- Domain data classes
DetailsArtist
andRelease
respectively representing a detailed artist and a release information; - A port
ArtistDetailsLoader
that will be consumed by the GUI to load the artist details. - A port
ArtistDetailsRemoteLoader
to fetch artist details from the instructure.
- Domain data classes
ui
package:- A view model
ArtistDetailsViewModel
that drives the domain by using the portArtistDetailsLoader
, transforms domain objects into displayable data and exposes this data; - A fragment
ArtistDetailsFragment
that will observe and display the displayable data fromArtistDetailsViewModel
.
- A view model
infra
package:MBArtistDetailsLoader
which implements the portArtistDetailsRemoteLoader
using the MusicBrainz API.
di
package:ArtistDetailsModule
is the dependency resolver.
Note: In the case of this little project, the port ArtistDetailsRemoteLoader
just reimplements the ArtistDetailsLoader
port because their abstraction is pretty-much equivalent but they could very well drift apart if future requirements make it necessary.
Important infrastructure libraries used in the project such as Retrofit and Realm are abstracted away from the domain which never uses them directly, thus avoiding to be polluted with non-agnostic elements (such as a Realm's context
or other properties) and allowing an easier library replacement if necessary.
Regarding Retrofit, the abstraction solely consists in the additional interface ArtistDetailsRemoteLoader
above MBArtistAPI
.
In the case of Realm, this is a bit more complex because I didn't want RealmResults
to leak outside of the infrastructure. That is the reason why I introduced RealmResultsLiveData
which, as its name suggests, is a LiveData
wrapper around RealmResults
.
In an ideal world, the domain is supposed to be dependant of nothing except the language itself of course & arguably that language's standard library. If we make this rule a hard rule, domain shouldn't depend on anything related to the Android SDK.
Nevertheless, I did made a compromise by using architecture component LiveData
. In my opinion, it's a pretty solid abstraction (although not being a fan of how it's tied up to lifecycle) and it's easily composable. I can see becoming it a "de facto" standard for the Android platform.
Speaking of observable data, I ruled Rx out since I felt the conjunction of Kotlin's coroutines & LiveData
would allow achieving the same goals.
For sure the throttling of the search text would have been more concisely written with Rx but why not avoiding such a large and complex third-party library if possible?
Regarding architecture components, I've also chosen to avoid Data Binding because I dislike having data-related stuff mixed up in what could have been a plain and stupid view. Not to mention the annoying back and forth switch between the code and the layout editor.
I've chosen to use dedicated domain objects instead of using Realm or MusicBrainz objects. While it certainly has a cost in terms of mapping, this allows independence from the infrastructure (yet again...) and refined domain representation. Indeed, backend and database objects might contain useless data, might use non-precise types, or might be structured in a way that does no represent key domain abstractions.
In my experience, such a decision as to be contrasted: if the backend is at our hand and allows us to describe exactly what we want, the domain object could be avoided. Regarding the database, industry major changes like the introduction of "recent" alternatives to historical libraries (CoreData or pure SQLite) such as Realm, or the brutal shut down of up and coming services such as Parse show us the importance of the abstraction.
A big advantage of using Realm in my book is the fact that it is multi-platform that why I tend to use it for applications that run on both iOS & Android. The modeling effort can be mutualized as well as the DAO interfaces.
On the other side, one advantage of Room is that it fits out-of-the-box with LiveData
. Should I have opted for Room, I wouldn't have had to make a wrapper for it.
Anyway, I'm sure Room would have been an excellent fit for the local database as well but I do not know much about it yet while I do know Realm so it made the choice easier.
Note: The database is not encrypted as no sensitive nor personal data is stored. But Realm provides a very simple encryption mechanism so all we would need to do would be to create an encryption key for instance with the KeyStore API.
-
Project dependencies are managed with the Gradle Kotlin DSL. This allows sharing configuration across modules. For instance, all the dependencies versions are declared in the
Dependencies.kt
file inbuildSrc
. -
I did not use the complete MusicBrainz API URL as the base Retrofit URL. This allows using the Wikipedia extract url MusicBrainz that does not seems to be available through the API. Also, this allow having a single Retrofit instance and having more control of which version to use for each one of the API.
-
In order to keep thing simple, I've deliberately chosen to share the
ArtistDetailsViewModel
with all artist details fragments rather than using a dedicated view model for each. Nevertheless, it would clearly require more separation would the number of tabs or their intrinsic complexity grows. -
While struggling to make unit tests work, I faced the decision of whether or not I should open classes to be able to mock them. I've finally chosen to rely on Dexopener that is able to re-open final classes at runtime, at the cost of a bit of configuration. While being advertised by the Mockk library, I wasn't so happy about introducing a dependency to such an unappreciated (93 stars on Github at the time). In the end, it's still a better option that opening stuff just for the sake of testing. I just hope this solution becomes more main-stream or that Mockk or Google eventually propose an alternative.
Retrofit: "de-facto" standard for HTTP API calls. Also compatible with Gson and coroutines through adapters.
Gson: JSON serialization / deserialization. I started with the up and coming Moshi during the test but faced an unexplained deserialization issue so I switched back to Gson.
Glide: high-performance & highly customizable image downloader. There are a lot of alternatives (Fresco, Picasso, …) out there but I sticked with the one I was the more comfortable with.
Mockito / Mockk: popular mocking. I started with Mockito, knowing it well, then I faced the issue of mocking final classes. A lot of people were suggesting using Mockk so I gave it a try and find it covers pretty much all use-cases I need from Mockito. It is still less popular than Mockito but given its API seems more Kotlin idiomatic, it might change at some point. Anyway, the sage is pretty similar so it won't take much work to migrate from one to another.
Koin: dependency injection. At first, I was hesitating to use Koin because I only have used Dagger in the past. Nevertheless given than I literally hated the Dagger experience, I made the move to Koin and didn't regret it at all. It's small, simple (at least for small projects like this one) and it works as expected.
Postman: to try out the MusicBrainz API
https://www.materialpalette.com: to define a color palette for the application
Working on a legacy Java project for the last couple years, I've seen this test as an opportunity to learn more on Kotlin and architecture components, that I never had the opportunity to really work with other than while following Google's codelabs. The learning curve is steep but I'm glad I did it although I wouldn't totally exclude using the "old" MVP approach again (for a couple of reasons explained below).
MusicBrainz API works just fine but the documentation is lacking. I searched over and over where I could get a precise model description, that would describe which field is mandatory or not. Instead of that, I've had to change a lot of properties that I genuinely thought were mandatory only to realize -while using the application- they were not. In short, I would have appreciated it if MusicBrainz provided a Swagger description file or a Postman collection.
I struggled with an API 19 incompatibility that was causing a crash at launch. It was revolving around Retrofit and the SSLSocketFactory
. Further investigation showed that is was rather the OkHttp library (a transitive dependency of Retrofit) version that was incompatible with API 19. I downgraded Retrofit version to a compatible one but it was still crashing. I then waste a lot of time trying to add boilerplate code (custom SSLSocketFactory or forcing an update through Google Play Services, as suggested on the internet) to make it work to finally realize the problem was coming from the Retrofit logging adapter library and not Retrofit itself.
I spend quite some time trying to fix a strange bug: when selecting an artist to see the details screen then coming back to the search screen and then finally selecting an artist again, the discography and about tabs only displayed blank content. Through debugging and logging, I detected that the lifecycle of both fragments was weird. The documentation of FragmentPagerAdapter
didn't help me at all. After trying a multitude of different ways to instantiate those fragments or to load their contents, I almost gave up before trying -without much hope- to use a FragmentStatePagerAdatper
instead. Fortunately, it worked, even though I didn't really spend more time to understand why.
I can't say it was a surprise, having already experienced it in the past, but the tests gave me a lot of headaches, especially instrumented ones.
Notably, while testing SearchArtistFragment
I faced the issue of unmockable final classes. As already explained, I didn't want to open everything just for testing, and at first, I didn't want to use Dexopener either.
So my reflex was to hide the SearchArtistViewModel
behind an interface, as I would have done using an MVP approach. The fragment only knowing that interface, that would have allowed me to mock it easily for the tests. But then Koin was in the way because viewModel() DSL expect an instance of ViewModel
. But getting rid of Koin wouldn't solve the problem either because as a matter of fact this only reflects a bigger issue with architecture components: ViewModelProvider
doesn't allow the use of an interface. In my opinion, this is a real design flaw and if I the motivation behind it (from my understanding: simplify the life of the developers especially regarding the lifecycle) is noble, it clearly stands in the way of decoupling and it only makes tests harder.
I also couldn't find a proper way to test:
FoundArtistsListAdapter
because I didn't find a way for the view holderitemView
to be mocked;MBArtistFinder
because I faced an issue with the conjunction ofLiveData
and coroutines;- Some functions of
RealmPreviousArtistSearchesDao
that requires Realm to be instantiated with a Looper and I didn't find a way to bypass this.
Anyway, I let those 3 unfinished test classes in the project just for the sake of the discussion.
- Ideally, the
searchartist
andartistdetails
should have been packaged into separated modules, along with their respective tests. This would have allows to make a lot of thingsinternal
instead ofpublic
thus hiding implementation details to other modules; - Manage no network status and networking errors;
- Add more tests;
- Fix the last FIXME / TODO.