Creating a maintainable, flexible codebase is not easy but is an essential part of software engineering. In this series we’ll take a look at a simple, functional weather app and look at some of the issues in its design. We shall then refactor and re-design it to create a codebase which will be easier to maintain, less prone to bugs, and easier to add features to. This series is not going to be a deep dive in to the techniques and technologies that we’re going to use, but will be more an exploration of what benefits they give us. In this article we’ll look at separating our concerns.
Previously we looked at some of the issues in a smile weather app, particularly with the Fragment class which displays the current weather. One of the big smells was that there were a huge number of imports (around 40 in a <200 line source file) which is a strong suggestion that that class was trying to collaborate with too many other components. It also made testing really difficult because everything was in a single class which was difficult to instantiate in a unit test suite. So breaking it up in to smaller components will certainly help this, but in doing so we want to separate our concerns in to small, manageable, and testable components. Let's take a look at a reallysimple thing that we can spin out in to a separate component: how we obtain the current location.
In CurrentWeatherFragment we use the FusedLocationProviderClient to obtain the location. This is actually a really clean and easy to use API and so it doesn’t, on the face of it, look like a big deal to just do everything here. The problems aren’t the amount of code required to use it, but that it is part of Google Play Services and this makes our Fragment tightly coupled to Play Services. If this were in the codebase of a commercial app this would be a really big problem. Play Services is only available on certified Android devices which have the Play Store app pre-installed, and so will not work if the app is released via other app stores. Even if the plans are to only ever release the app through the Google Play Store sometimes companies can make commercial decisions to change such things, so having such a tightly coupled solution would make releasing the app though a different app store quite a bit of extra work to switch to an alternate location provider. However, if we abstract away the details of the implementation then we could potentially make the change to a different location provider a whole lot easier.
Let’s start by creating a simple interface:
package com.stylingandroid.weatherstation.location
interface LocationProvider {
fun requestUpdates(callback: (latitude: Double, longitude: Double) -> Unit)
fun cancelUpdates(callback: (latitude: Double, longitude: Double) -> Unit)
}
This is a really simple, clean publish / subscribe API. To start receiving location updates it we do the following:
override fun onResume() {
super.onResume()
context?.apply {
if (this is AppCompatActivity) {
supportActionBar?.apply {
title = getString(R.string.current_weather)
setHasOptionsMenu(true)
setDisplayHomeAsUpEnabled(false)
}
}
locationProvider.requestUpdates(::retrieveForecast)
}
}
private fun retrieveForecast(latitude: Double, longitude: Double) {
currentWeatherProvider.request(latitude, longitude, ::bind)
}
And to stop receiving updates:
override fun onPause() {
locationProvider.cancelUpdates(::retrieveForecast)
super.onPause()
}
So none of the implementation details of how the location is actually retrieved is exposed by this interface. Obviously we need a concrete implementation of this:
internal class FusedLocationProvider(
private val context: Context,
private val fusedLocationProvider: FusedLocationProviderClient =
LocationServices.getFusedLocationProviderClient(context)
) : LocationProvider {
private val permissions: Array<out String> = arrayOf(
Manifest.permission.ACCESS_FINE_LOCATION,
Manifest.permission.ACCESS_COARSE_LOCATION
)
private val subscribers = mutableListOf<(latitude: Double, longitude: Double) -> Unit>()
override fun requestUpdates(callback: (latitude: Double, longitude: Double) -> Unit) {
if (subscribers.isEmpty()) {
subscribers += callback
if (permissions.all { context.checkSelfPermission(it) == PERMISSION_GRANTED }) {
LocationRequest.create().apply {
fusedLocationProvider.requestLocationUpdates(this, locationCallback, null)
}
}
} else {
subscribers += callback
}
}
override fun cancelUpdates(callback: (latitude: Double, longitude: Double) -> Unit) {
subscribers -= callback
if (subscribers.isEmpty()) {
fusedLocationProvider.removeLocationUpdates(locationCallback)
}
}
private val locationCallback = object : LocationCallback() {
override fun onLocationResult(locationResult: LocationResult?) {
locationResult?.locations?.firstOrNull()?.also { location ->
val (latitude, longitude) = location
subscribers.forEach { callback ->
callback(latitude, longitude)
}
}
}
}
}
This implementation not only hides the actual location provider that we’re using, it also implements the publish / subscribe logic with appropriate checks that we have the necessary permissions. It is not the responsibility of this component to request permissions from the user – that is for the UI to do, but here we just check that the relevant permissions have been granted before we attempt to request location updates from the FusedLocationProviderClient.
There’s one thing worth noting here: The default constructor argument for the FusedLocationProviderClient instance. By using a default constructor argument to instantiate this then it means that we can create an instance of this class with a single Context argument, and this is how it will be used when created in production code. However, having this as an optional constructor argument enables us to also pass in a mocked FusedLocationProviderClient and suddenly this class becomes much easier to unit test. The test suite does not test the behaviour of the underlying LocationProvider, but the subscribe / unsubscribe and callback logic that is the responsibility of this class.
So now if we needed to substitute in an alternate location provider we could create a different concrete implementation of LocationProvider, and have the two different implementations in separate source sets for different build flavours. That will enable us to create distinct APKs which include different location providers, with and identical Fragment implementation in each.
We can apply exactly the same principles to the component which obtains the weather data: Create a simple, clean interface to abstract the behaviour, and create a concrete implementation of that which moves all knowledge of which weather provider we’re using (and even that we’re using Retrofit to handle the network calls, and Moshi to marshall the Json response body to Java objects. For brevity I won’t include the full source here, but it’s available in the source project.
There’s one important addition to the weather data implementation. Previously we passed the object that was marshalled from Json by Moshi to the Fragment to display. However this structure was dictated my the hierarchical structure defined within the OpenWeatherMap API. So by using this we were implicitly keeping a dependency on to OpenWeatherMap as a result. To remove this I created a new data class named CurrentWeather:
data class CurrentWeather(
val latitude: Float,
val longitude: Float,
val placeName: String,
val temperature: Float,
val windSpeed: Float,
val windDirection: Float,
val weatherType: String,
val weatherDescription: String,
val icon: String,
val timestamp: Instant
)
The structure marshalled from Json then gets converted to this object before it is passed to the Fragment to display. This serves two main purposes: The first is to abstract us from the OpenWeatherMap API structure – which makes it easier to switch providers; and the second is that it removes the information that we’re not interested in – making us more memory efficient. There are some additional benefits from this which we’ll see later in the series.
Performing a conversion of this type is effectively switching data from one domain model to another. The source data is defined by OpenWeatherMap and we can’t easily change that. The new CurrentWeather object is our own internal domain model and is what we require to be able to display the information that we’re interested in.
Once again, providing a clean abstraction also permits us to create tests far more easily. In this case it is more of an integration test than a unit test because it is also testing our Json to data class conversion as well as the locic contained within OpenWeatherMapProvider. If this were a production app I would also wish to simulate different network conditions so that our test suite could test how the code responds to changing network status. However for time & brevity I have skipped that in this case.
Turning our attention back to our CurrentWeatherFragment:
package com.stylingandroid.weatherstation.ui
import android.annotation.SuppressLint
import android.content.Context
import android.os.Bundle
import android.view.LayoutInflater
import android.view.Menu
import android.view.MenuInflater
import android.view.MenuItem
import android.view.View
import android.view.ViewGroup
import androidx.appcompat.app.AppCompatActivity
import androidx.fragment.app.Fragment
import androidx.fragment.app.transaction
import androidx.transition.TransitionManager
import com.stylingandroid.weatherstation.BuildConfig
import com.stylingandroid.weatherstation.R
import com.stylingandroid.weatherstation.location.FusedLocationProvider
import com.stylingandroid.weatherstation.location.LocationProvider
import com.stylingandroid.weatherstation.model.CurrentWeather
import com.stylingandroid.weatherstation.net.CurrentWeatherProvider
import com.stylingandroid.weatherstation.net.OpenWeatherMapProvider
import kotlinx.android.synthetic.main.fragment_current_weather.*
class CurrentWeatherFragment : Fragment() {
private lateinit var locationProvider: LocationProvider
private lateinit var currentWeatherProvider: CurrentWeatherProvider
private lateinit var converter: Converter
override fun onCreateOptionsMenu(menu: Menu?, inflater: MenuInflater?) {
inflater?.inflate(R.menu.main_menu, menu)
super.onCreateOptionsMenu(menu, inflater)
}
override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?
): View? = inflater.inflate(R.layout.fragment_current_weather, container, false)
override fun onAttach(context: Context) {
super.onAttach(context)
locationProvider = FusedLocationProvider(context)
currentWeatherProvider = OpenWeatherMapProvider(context, BuildConfig.API_KEY)
converter = Converter(context)
}
@SuppressLint("MissingPermission")
override fun onResume() {
super.onResume()
context?.apply {
if (this is AppCompatActivity) {
supportActionBar?.apply {
title = getString(R.string.current_weather)
setHasOptionsMenu(true)
setDisplayHomeAsUpEnabled(false)
}
}
locationProvider.requestUpdates(::retrieveForecast)
}
}
private fun retrieveForecast(latitude: Double, longitude: Double) {
currentWeatherProvider.request(latitude, longitude, ::bind)
}
private fun bind(current: CurrentWeather) {
TransitionManager.beginDelayedTransition(content_panel)
all_widgets.visibility = View.VISIBLE
city.text = current.placeName
temperature.text = converter.temperature(current.temperature)
wind_speed.text = converter.speed(current.windSpeed)
wind_direction.rotation = current.windDirection
timestamp.text = converter.timeString(current.timestamp)
summary.text = current.weatherType
type_image.contentDescription = current.weatherDescription
type_image.setImageResource(
resources.getIdentifier(
"ic_${current.icon}",
"drawable",
context?.packageName
)
)
}
override fun onOptionsItemSelected(item: MenuItem): Boolean =
when (item.itemId) {
R.id.action_settings -> {
fragmentManager?.transaction(allowStateLoss = true) {
replace(R.id.activity_main, PreferencesFragment())
addToBackStack(PreferencesFragment::class.java.simpleName)
}
true
}
else -> super.onOptionsItemSelected(item)
}
override fun onPause() {
locationProvider.cancelUpdates(::retrieveForecast)
super.onPause()
}
}
The first thing to note is that our imports look much healthier now. Not only have we halved them, but they are all much more related to UI work rather than the complete mixture we had before. In addition to that we have a much simpler Fragment which, once again, is much more focused on the UI.
Our code is now more testable and the tests that I added have made a marked improvement on the coverage:
While code coverage should be used carefully because it can be misleading, seeing upward trends in code coverage is more often than not a good thing! Another point which is worthy of note is that all of these tests are JUnit tests because our abstractions have been to simple JVM object rather than having logic inside Android Framework components – which would require Espresso tests or Robolectric. As a result they run pretty quickly so don’t vastly increase our build times.
However there are still some issues. The first is that we have to call the constructors of our concrete implementations of LocationProvider and WeatherProvider (highlighted in the code sample). Another issue is the constructor of OpenWeatherMapProvider:
class OpenWeatherMapProvider(
context: Context,
private val appId: String,
okHttpClient: OkHttpClient = OkHttpClient.Builder()
.cache(Cache(context.cacheDir, cacheSize))
.addInterceptor(HttpLoggingInterceptor().apply {
level = HttpLoggingInterceptor.Level.BODY
})
.build(),
converterFactory: Converter.Factory = MoshiConverterFactory.create(
Moshi.Builder()
.add(KotlinJsonAdapterFactory())
.build()
),
retrofit: Retrofit = Retrofit.Builder()
.client(okHttpClient)
.baseUrl("https://api.openweathermap.org/")
.addConverterFactory(converterFactory)
.build(),
private val service: OpenWeatherMap = retrofit.create(OpenWeatherMap::class.java),
private val calls: MutableList<Call<Current>> = mutableListOf()
) : CurrentWeatherProvider {
...
}
I’ve used the same default argument approach that I did for FusedLocationProvider, but creating the OpenWeatherMap instance is a bit more complex and has resulted in that monstrosity of a constructor. While we could abstract that out to object factories this is also a case where dependency injection may help us and, in the next article in this series, we’ll look at the benefits of using that.
The source code for this article is available here.
© 2018, Mark Allison. All rights reserved.
Copyright © 2018 Styling Android. All Rights Reserved.
Information about how to reuse or republish this work may be available at http://blog.stylingandroid.com/license-information.
