Maintainable Architecture – Readability

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 how we can improve the readability of our code.

In the last few articles in this series we’ve added some new features, but we’ve also added some technical debt. One of the big problems that we’ve introduced is that there is a chunk of the new code which is difficult to read and understand. While this may not seem like a big deal – particularly if the code works, it can actually cause us a major headache further down the line. The problem is that if a piece of code is difficult to read and understand, then it is also harder to bug fix if bugs are later found; and it is also harder to extend and add new features and behaviours to. In other words, difficult to read code is difficult to maintain, and therefore is somewhat at odds with the whole purpose of this series of articles.

It is always worth giving a thought to whomever may have to maintain your code in the future. Not only is it the responsible thing to do, but bear in mind that the person who may have to maintain your unreadable code could be you!
Just because you understood the code when you wrote it, it may be much harder to understand it after a few months or even years when you have to change it.

Let’s take a look at a specific piece of code that I’m particularly concerned about. Back in the article on the data layer for the five day forecast we added a conversion function:

private fun dailyItems(items: List<WeatherForecastItem>): List<FiveDayForecast.DailyItem> =
        items.groupBy { it.timestamp.atZone(ZoneId.systemDefault()).toLocalDate() }
                .filter { it.value.size == 8 }
                .map {
                    FiveDayForecast.DailyItem(
                            it.key,
                            it.value
                                    .groupBy { it.weatherType }
                                    .entries
                                    .sortedByDescending { it.value.size }
                                    .first()
                                    .value
                                    .first()
                                    .weatherType,
                            it.value
                                    .groupBy { it.icon.dropLast(1) }
                                    .entries
                                    .sortedByDescending { it.value.size }
                                    .first()
                                    .let {
                                        "${it.key}d"
                                    },
                            it.value.sumByDouble { it.windSpeed.toDouble() }
                                    .toFloat() / 8f,
                            (it.value.sumByDouble { it.windDirection + 360.0 }
                                    .toFloat() / 8f) - 360f,
                            it.value.maxBy { it.temperatureMax }?.temperatureMax ?: 0f,
                            it.value.minBy { it.temperatureMin }?.temperatureMin ?: 0f
                    )
                }

Although this code is quite clever and does an awful lot in relatively few lines of code it is pretty difficult to understand, and it took quite a bit of explanation in the article in which it appeared. Having to alter, extend, or bug fix this could well be a problem because the cognitive effort to understand precisely what it’s doing is quite large. Moreover, because of the lack of readability it is more that there will be misunderstandings in precisely what it is doing and bug could be introduced as a result when it is changed in the future.

To improve the readability, we need to consider what it is actually doing, and break it down in to smaller, easier to understand chunks. Let’s start by considering what the function actually does: It takes a list of WeatherForecastItem objects, and converts them in to a list of FiveDayForecast.DailyItem instances. However it is not a one-to-one mapping because a DailyItem is an aggregation of the data from each of the WeatherForecastItem items for any specific day. So let’s simplify the method:

private fun dailyItems(items: List<WeatherForecastItem>): List<FiveDayForecast.DailyItem> =
        items.forecastsGroupedByDay()
                .map(Map.Entry<LocalDate, List<WeatherForecastItem>>::toDailyItem)

That’s much simpler, and the body of the function now gives a clear understanding of what the conversion is. We first group the items by day (line 67) and then transform them by mapping each entry in that grouped list to a DailyItem (line 68). For the sake of keeping ForecastRespository readable, and focused on its own responsibilities, we’ll move the actual conversion logic elsewhere. This not only separates our concerns, and keeps things focused, it also creates a nice separation point where we could add unit test around the conversion logic so that it can be tested independently – which is also good for maintainability because if the code has good tests and we alter it later on, then the tests give us confidence that we haven’t introduced any bugs when we come to change something.

We’ll move all of the conversion logic in to a file named ForecastConverter, which will mainly be Kotlin extension function (which can make things much easier for us):

fun List<WeatherForecastItem>.forecastsGroupedByDay(): Map<LocalDate, List<WeatherForecastItem>> {
    return groupBy { it.timestamp.atZone(ZoneId.systemDefault()).toLocalDate() }
            .filter { it.value.size == 8 }
}

fun Map.Entry<LocalDate, List<WeatherForecastItem>>.toDailyItem(): FiveDayForecast.DailyItem {
    return FiveDayForecast.DailyItem(
            key,
            value.mostCommonWeatherType,
            value.mostCommonWeatherIcon,
            value.averageWindSpeed,
            value.averageWindDirection,
            value.maximumTemperature,
            value.minimumTemperature
    )
}

These are the two functions that we called from ForecastRespoitory.dailyItems(). Each of these does one specific task, and the name of the function describes that task pretty well, I think.

forecastsGroupedByDay() will group the WeatherForecastItems in the List according to the day, and then filter so that only the days which have a full 8 forecasts are returned in the list.

toDailyItem() is an extension on Map.Entry<LocalDate, List<WeatherForecastItem>>, and will create a new DailyItem based upon the key (a LocalDate representing the day) and value (a List<WeatherForecastItem&gt representing the forecasts for that day), and creates a new FiveDayForecast.DailyItem. Once again with some careful naming, most of the arguments to the DailyItem constructor are quite easy to understand. The properties that we access are themselves Kotlin extension properties:

private val List<WeatherForecastItem>.mostCommonWeatherType: String
    get() {
        return groupBy { it.weatherType }
                .entries
                .sortedByDescending { it.value.size }
                .first()
                .value
                .first()
                .weatherType
    }

private val List<WeatherForecastItem>.mostCommonWeatherIcon: String
    get() {
        return groupBy { it.icon.dropLast(1) }
                .entries
                .sortedByDescending { it.value.size }
                .first()
                .let {
                    "${it.key}d"
                }
    }

private val List<WeatherForecastItem>.averageWindSpeed: Float
    get() = sumByDouble { it.windSpeed.toDouble() }.toFloat() / 8f

private val List<WeatherForecastItem>.averageWindDirection: Float
    get() = (sumByDouble { it.windDirection + 360.0 }.toFloat() / 8f) - 360f

private val List<WeatherForecastItem>.maximumTemperature: Float
    get() = maxBy { it.temperatureMax }?.temperatureMax ?: 0f

private val List<WeatherForecastItem>.minimumTemperature: Float
    get() = maxBy { it.temperatureMax }?.temperatureMax ?: 0f

I will not give a detailed explanation of each of these because the actual logic was explained in the earlier article. However, it is worth considering that although they each contain some logic which requires some study to property understand, each of these extension properties in isolation requires far less cognitive effort than the unwieldy chunk of code that we started with at the beginning of this article. Furthermore if, for example, we only needed to tweak the averageWindDirection logic, then finding that an understanding it is much simpler than in the block we started with. The division in to smaller components and the descriptive naming make each block of code easier to take in, and it is much easier to ignore the bits that we’re not interested in.

In closing, it is worth further stressing the importance of good naming. Choosing a descriptive name for a class, function or variable which clearly communicates its purpose can have a massive impact on how easy it is to read your code. That’s not to say that good naming is easy – it’s not, but by giving careful thought to naming, and also being mindful of renaming things if their purpose changes can make your code much easier to maintain. And you will one day thank yourself for it because you’ll be able to understand that bit of code you wrote two years ago!

We’re almost done with this series on Maintainable Architecture, but there’s still one more thing that we’ll look at. The UI logic is pretty UI focused except for how we switch from one Fragment to another. For example: CurrentWeatherFragment has to create instances of DailyForecastFragment in order to switch between Fragments following a user tap. This is a poor separation of concerns, so in the final article we’ll look at how we can overcome 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.