I prefer not to use the keyword “it”

And the reason is simple:

I want to be as explicit as possible and allow the reader of my code to have an uninterrupted flow.

Think about it. Every time you encounter the it keyword you do, a quick, conversion between what you see and what it represents. Personally I do it even in very small lambdas, imagine if you are two or three lines deep in a lambda and you see an it:

val animals = listOf("Lion", "Penguin", "Giraffe", "Whale", "Shark")
val usernames = animals.map {
val randomNumber = Random.nextInt(0, 10)
val randomCharacter = listOf("!", "@", "$", "%", "#")[Random.nextInt(0, 5)]
"$it$randomCharacter$randomNumber"
}
view raw prefer_not_using_it__non_usage.kt hosted with ❤ by GitHub

It might not look much in this simple example but read it now with an explicit value:

val animals = listOf("Lion", "Penguin", "Giraffe", "Whale", "Shark")
val usernames = animals.map { animalAsBase ->
val randomNumber = Random.nextInt(0, 10)
val randomCharacter = listOf("!", "@", "$", "%", "#")[Random.nextInt(0, 5)]
"$animalAsBase$randomCharacter$randomNumber"
}
view raw prefer_not_using_it__usage.kt hosted with ❤ by GitHub

You don’t have to do a mental translation and it also provides some details regarding the format of username.

This last part can make the code even more readable since it allows us to describe the values we use:

values.map { rawValue -> Name.of(rawValue) }
view raw prefer_not_using_it__usage_2.kt hosted with ❤ by GitHub

this hints that (a) values list does not contain usable data and (b) the of function will perform some kind of cleaning

Don’t share constants between production and test code

Building upon my previous post and the trick of being specific in the values the code respects, one pattern that I’ve noticed which can easily lead in many false positive tests is sharing a constant value between production and test code.

If the test code reads the value from the production, any change that was done by mistake will not affect the test which will continue to pass!

21 yeas of age

Lets say that we have two services, one checks if a customer can enter a casino and the other if she can buy alcohol. For both cases the law states that the minimum legal age is 21 years old.

The code has a configuration file, a domain and two modules for each service:

// Production code:
// configuration
object Config {
const val MIN_LEGAL_AGE = 21
}
// domain
class Person(val age: Int)
// entrance module
fun canEnterCasino(person: Person): Boolean {
return person.age >= Config.MIN_LEGAL_AGE
}
// alcohol module
fun canBuyAlcohol(person: Person): Boolean {
return person.age >= Config.MIN_LEGAL_AGE
}
// Test code:
// entrance module
fun `a customer can enter the casino when she is older than 21 years of age`() {
val twentyOneYearOld = Person(Config.MIN_LEGAL_AGE)
val actual = canEnterCasino(twentyOneYearOld)
assertTrue(actual)
}
// alcohol module
fun `a customer can buy alcohol when she is older than 21 years of age`() {
val twentyOneYearOld = Person(Config.MIN_LEGAL_AGE)
val actual = canBuyAlcohol(twentyOneYearOld)
assertTrue(actual)
}
view raw dont_share_constants__sharing.kt hosted with ❤ by GitHub

As you can see the tests consume the minimum age directly from the production code but the test suite passes, life is good.

Then one day, the law changes and the minimum legal age for entering a casino drops to 20 years! Simple change, not much of a challenge for the old timers so the task is being given to the new teammate who does not know all modules yet and is also a junior software engineer.
She sees the test, changes the value in the name to 20, sees the config, changes the constant’s value to 20, runs the test suite, everything passes, life is good! Only that it isn’t because the casino’s software now allows selling alcohol to 20 year olds!

Keep them separate

If the test code did not use the production’s code

// Production code:
// configuration
object Config {
const val MIN_LEGAL_AGE = 20
}
// domain
class Person(val age: Int)
// entrance module
fun canEnterCasino(person: Person): Boolean {
return person.age >= Config.MIN_LEGAL_AGE
}
// alcohol module
fun canBuyAlcohol(person: Person): Boolean {
return person.age >= Config.MIN_LEGAL_AGE
}
// Test code:
// entrance module
fun `a customer can enter the casino when she is older than 20 years of age`() {
val twentyOneYearOld = Person(20)
val actual = canEnterCasino(twentyOneYearOld)
assertTrue(actual) // passed
}
// alcohol module
fun `a customer can buy alcohol when she is older than 21 years of age`() {
val twentyOneYearOld = Person(21)
val actual = canBuyAlcohol(twentyOneYearOld)
assertTrue(actual) // failed
}
view raw dont_share_constants__not_sharing.kt hosted with ❤ by GitHub

then, after changing the constant’s value, the test suite would fail alerting the software engineer that something has broken forcing her to figure it out and craft another solution.

Your tests can also be your documentation

Tests help as make sure that our code works, provide us a safety net when we need to refactor and, when having proper test names, can be a good documentation describing what the code does.
The last one can be especially helpful for both newcomers that need to understand the system and old timers that haven’t visited the code for a while!

A couple of tricks for achieving good names are:

  1. Avoid describing how the code does something and try to describe what it does
    For example:
    calling add(item) results in calling recalculate
    is way too specific without providing anything meaningful, or anything that we wouldn’t get from reading the code.
    On the other hand:
    a recalculation of the order's value takes place every time a new item gets added
    shares an important information about the Order‘s behavior when adding an item.
  2. Avoid being too abstract
    For example:
    a customer can buy alcohol when she is of legal age
    can help the reader understand how the code behaves but in a documentation you need specific values.
    So:
    a customer can buy alcohol when she is older than 21 years of age
    is much better because it also provides the exact threshold that our code considers for allowing someone to buy alcohol

Two reasons why you should add a return type in your functions in Kotlin

Kotlin’s compiler is clever enough to figure out on its own what is the return type of a function but this does not mean that we should over use it and here is why:

Help the compiler to help us

By adding a return type in a function we instruct the compiler to expect and force that type (by a compiler error). On the other hand if we allow the compiler to infer the type, if something changes in the function’s body and the return type is not the one intended by the author, the compiler will follow along thinking that we know what we are doing!

Help the reader [to help us]

We write something once but it gets read multiple times. So it is our responsibility to make it as readable, explicit and quick in the eye as we can. In every case that we omit a return type the reader of our code has to do the calculations and extract the type on her own which, depending on the complexity, will take time and effort. You might say that a few seconds is not a big deal but in comparison with the zero seconds of having a type it is a lot.

Also not all reading takes place in an IDE that provides hints and colorful help. Our PR reviewers will probably read out code directly from GitHub or GitLab. By making them change context to figure something out we break their flow and concentration.

So, when should we use it

Never! In my humble opinion the only valid place is in small (one line), private methods that either construct something, so the use of the constructor along side the = sign trick the mind:

private fun createName(value: String) = Name(value)
view raw use_return_type__createname.kt hosted with ❤ by GitHub

or the return type is clearly obvious:

private fun isCompleted(task: Task) = task.status in listOf(COMPLETED, CANCELLED)
view raw use_return_type__iscompleted.kt hosted with ❤ by GitHub

Don’t force your objects to construct what they need

Let’s say we have an object that handles instances of Person. For example PeopleScreen:

// Person.kt
class Person(
val name: String,
val surname: String
)
// PeopleScreen.kt
class PeopleScreen(
private val people: List<Person>
) {
fun render() {
people.forEachIndexed { index, person ->
println("${index + 1}. ${person.name}, ${person.surname}")
}
}
}
// Usage:
fun main() {
val people = listOf(
Person("Joe", "Dow"),
Person("Jill", "Doe"),
Person("Jack", "Black")
)
val screen = PeopleScreen(people)
screen.render()
}
view raw dont_force_construction__the_proper_way.kt hosted with ❤ by GitHub

PeopleScreen renders instances of Person so this should be the only format we provide to it. Let me explain.

Forced construction

There is a new flow that ends in opening PeopleScreen but all the information for the list of people are in a Map<String, String>. There is no reason to alter PeopleScreen in order to support this new format:

// Person.kt
class Person(
val name: String,
val surname: String
)
// PeopleScreen.kt
class PeopleScreen(
private val people: List<Person>
) {
constructor(people: Map<String,String>) : this(
people.map { entry -> Person(entry.key, entry.value) }
)
fun render() {
people.forEachIndexed { index, person ->
println("${index + 1}. ${person.name}, ${person.surname}")
}
}
}
// Usage:
fun main() {
val people = mapOf(
"Joe" to "Dow",
"Jill" to "Doe",
"Jack" to "Black"
)
val screen = PeopleScreen(people)
screen.render()
}
view raw dont_force_construction__the_forced_way.kt hosted with ❤ by GitHub
the new format is passed through an overloaded constructor but the same goes if we use a setter method

Why we shouldn’t do it

We could argue that by doing so we tie the object with each special format making the code hard to maintain and scale but the real reason is that we violate the SRP principle since PeopleScreen will have more than one reasons to change. One if something changes in the way we render and two if something changes in Person‘s construction.

What we should do

We should keep PeopleScreen only consuming Person and move all transformations to their own objects allowing a coordinator to transform and pass data around.

Know your tools: $SELECTION$ in Intellij IDEA

I have used and created Live Templates before but I didn’t know about the special keyword $SELECTION$. I found out after reading IntelliJ IDEA / Android Studio Tricks: Surround With by Ivan Morgillo.

In short, when a template gets invoked, $SELECTION$ gets replaced by whatever is selected at that moment.

I won’t go into details about creating a new template. You can read all about it at Ivan’s post. But when you learn how to create one then add the following:

This way you can do something like this:

PS: the $END$ keyword is another special one that pinpoints where the cursor will stop after adding values for an invoked template

Good practices: First write the test then fix the bug

You get a report about a bug. You open the app, follow the steps to reproduce it and, as mentioned in the report, your app is misbehaving. What’s next?

You can either dig immediately in the code and fix the bug or you can re-reproduce the bug, only this time in a test. The second. Always go with the second option.

Here is why:

  1. From now on you will have a regression test.
    Meaning that if a change in the code breaks what you fixed you’ll get notified from the test suite and not your users
  2. It keeps you focused / You know when you finished.
    This is a benefit you get from TDD in general. When the test passes the bug is fixed and you can move to your next task. Also, since you have to make the test pass, anything else that popped up during your research for the bug can wait (I usually write it down to a notepad I keep next my keyboard).
  3. You get a better understanding of the code.
    By trying to write the test you get a better knowledge of how things are connected and communicate. Especially if you are new to a project this will boost your understanding significantly.
  4. You discover more corner cases.
    There are times that by writing this one test and seeing what inputs a class/function can have, you wonder how will the app behave under certain values. Finish the task at hand and then add a test for each case you want to explore. You might end up solving more bugs!

TIL: @NullAndEmptySource in JUnit5

This is a clear case of RTFM!

I wanted to make sure that a function will return null when given a null or empty string and what I ended up doing was something like this:

internal class CreateNameTest {
@ParameterizedTest
@ValueSource(strings = ["null", "", " "])
fun `there is no creation when the provided value is null or empty or blank`(providedValue: String?) {
val value: String? = if (providedValue == "null")
null else
providedValue
assertThat(createName(value), absent())
}
}
view raw initial_parameterized_test.kt hosted with ❤ by GitHub

@ValueSource does not accept null values so I passed it indirectly!

I didn’t like it so after, finally, reading the JUnit5 documentation I learned that the library had me covered from the beginning by providing three annotations exactly for this use case:

@NullSource, @EmptySource and @NullOrEmptySource are meant to be used whenever we need to check the behavior of our code when given null or empty inputs.

So the test changes to:

internal class CreateNameTest {
@ParameterizedTest
@NullAndEmptySource
@ValueSource(strings = [" "])
fun `there is no creation when the provided value is null or empty or blank`(providedValue: String?) {
assertThat(createName(providedValue), absent())
}
}
view raw parameterized_test_with_proper_annotations.kt hosted with ❤ by GitHub

PS: for the curious, the absent() is part of the hamkrest library by Nat Pryce (yes, that Nat Pryce)

A use case of using chain of responsibility pattern to scale strategy

Lets say we have a simple application that executes certain actions dictated by an external service.

More particular the external service sends a response that looks like this:

class Response(
val action: String,
val value: String
)
view raw response_first_iteration.kt hosted with ❤ by GitHub

the supported actions are:

interface Action {
val name: String
fun execute(response: Response)
}
class Print : Action {
override val name: String = "print"
override fun execute(response: Response) {
println("printing ${response.value}…")
}
}
class Log : Action {
override val name: String = "log"
override fun execute(response: Response) {
println("logging ${response.value}…")
}
}
class WriteToFile : Action {
override val name: String = "write_to_file"
override fun execute(response: Response) {
println("writing to file ${response.value}…")
}
}
view raw actions_first_iteration.kt hosted with ❤ by GitHub

and there is an executor to help as translate the response to an actual action execution:

class ActionExecutor {
private val allActions = mutableMapOf<String, Action>()
fun addAction(action: Action) {
allActions[action.name] = action
}
fun executeByResponse(response: Response) {
val action = allActions[response.action]
action?.execute(response)
}
}
view raw action_executor_first_iteration.kt hosted with ❤ by GitHub

This is an implementation of the Strategy Pattern where every strategy is a certain action allowing us to add as many actions as we want as long as there is a one-to-one relation with what the response sends:

fun main() {
val executor = createActionExecutor()
executor.executeByResponse(Response("print", "leonidas"))
executor.executeByResponse(Response("log", "kotlin"))
executor.executeByResponse(Response("write_to_file", "leonidas loves kotlin"))
}
private fun createActionExecutor(): ActionExecutor {
return ActionExecutor().apply {
addAction(Print())
addAction(Log())
addAction(WriteToFile())
}
}
view raw main_first_iteration.kt hosted with ❤ by GitHub

The problem

One day the external service decides to add a print error action but it does not do it by having a new action value. Instead it decides to have an is error flag which must be taken under consideration when the action value is print!

So the response is now this:

class Response(
val action: String,
val value: String,
val isError: Boolean = false
)
view raw response_second_iteration.kt hosted with ❤ by GitHub

leading us into having an if in the print action’s code

class Print : Action {
override val name: String = "print"
override fun execute(response: Response) {
if (response.isError) {
System.err.println("printing (in error stream) ${response.value}…")
return
}
println("printing ${response.value}…")
}
}
view raw print_second_iteration.kt hosted with ❤ by GitHub

which defeats the purpose of the strategy pattern and violates the SRP. Each strategy should do one thing and one thing only. By having an if in the strategy we allow it to implement two actions thus having two reasons to change.

Adding a chain

What we need is a way to allow the representation of a single action value from multiple strategies where each one implements a unique action depending on the values of the response.

This is where the Chain of Responsibility Pattern comes in helping as into having multiple actions tied together, passing the response to each other until one of them can execute it.

There are two ways to implement the pattern depending on the size and state of your project.

Using inheritance

If you have a small project with just a few actions and you don’t mind touching many files you can use inheritance and have each action containing the next in the chain:

abstract class BaseAction(
private val nextAction: Action?
) : Action {
override fun execute(response: Response) {
nextAction?.execute(response)
}
}
class Print : Action {
override val name: String = "print"
override fun execute(response: Response) {
println("printing ${response.value}…")
}
}
class PrintInErrorStream(
nextAction: Action?
) : BaseAction(nextAction) {
override val name: String = "print"
override fun execute(response: Response) {
if (response.isError) {
System.err.println("printing (in error stream) ${response.value}…")
return
}
super.execute(response)
}
}
view raw cof_inheritance.kt hosted with ❤ by GitHub

which can be used as such:

fun main() {
val executor = createActionExecutor()
executor.executeByResponse(Response("print", "leonidas"))
executor.executeByResponse(Response("log", "kotlin"))
executor.executeByResponse(Response("write_to_file", "leonidas loves kotlin"))
executor.executeByResponse(Response("print", "$&#$@#", isError = true))
}
private fun createActionExecutor(): ActionExecutor {
return ActionExecutor().apply {
addAction(PrintInErrorStream(Print())) // first try to print in error and the in simple
addAction(Log())
addAction(WriteToFile())
}
}
view raw main_cor_inheritance.kt hosted with ❤ by GitHub
Using composition

If we do not wish to change any of the existing actions then composition is the only way.

What we need is a way to wrap an existing action in an object that will (a) hold the next action in the chain and (b) decide which action will be executed, the wrapped or the next one:

class ChainAction(
private val action: Action,
private val canExecute: (Response) -> Boolean
) : Action {
private var nextAction: Action? = null
override val name: String = action.name
override fun execute(response: Response) {
if (canExecute(response)) {
action.execute(response)
return
}
nextAction?.execute(response)
}
fun or(action: ChainAction): ChainAction {
nextAction = action
return this
}
}
fun Action.asChain(canExecute: (Response) -> Boolean): ChainAction =
ChainAction(this, canExecute)
view raw chain_action.kt hosted with ❤ by GitHub

Using ChainAction results in this:

fun main() {
val executor = createActionExecutor()
executor.executeByResponse(Response("print", "leonidas"))
executor.executeByResponse(Response("log", "kotlin"))
executor.executeByResponse(Response("write_to_file", "leonidas loves kotlin"))
executor.executeByResponse(Response("print", "$&#$@#", isError = true))
}
private fun createActionExecutor(): ActionExecutor {
val printInErrorStream = PrintInErrorStream().asChain { response -> response.isError }
val justPrint = Print().asChain { true }
return ActionExecutor().apply {
addAction(printInErrorStream.or(justPrint))
addAction(Log())
addAction(WriteToFile())
}
}
view raw main_cor_composition.kt hosted with ❤ by GitHub

This way we can scale the strategy pattern both vertically, one strategy per action value, and horizontally, one strategy per action value sub cases.

I wrote a GitHub Action using Kotlin

I decided to take a look at GitHub Actions so for the past week I’ve been watching and reading everything about it. I even wrote a post, an Introduction to GitHub Actions. What I found really interesting is the fact that you can write an action using the language you feel more comfortable with. And so I did!

I wrote a small action that:

  1. collects all the changed files from a PR,
  2. keeps those that have the .kt suffix,
  3. runs ktlint on them and
  4. makes a comment in the PR for every error that ktlint reports

and all that using Kotlin and some bash!

You can find and use it here: ktlint-pr-comments

GitHub Action using Docker

There are three ways to create an action but only the one using Docker allows us to use the language we want.

In all three ways the main two ingredients are:

  • the action’s code
  • the action’s metadata, a file called action.yml which is placed in the root folder of your project and defines how the action will run and what inputs/outputs it has.

In our case there is also a third ingredient, a Dockerfile or a docker image which is used by the action’s runner to create a container and execute the action’s code inside it. All you have to do is to make sure that the action’s executable parts are being copied in the container and that are called upon its start.

The runner makes sure that the working space is being mounted to the container (in the state that it was just before the action is started) along with all the environment variables and the inputs the action needs. You can read more in the documentation.

Ktlint PR comments

Action’s code

The action has three distinct parts.

The first part is responsible for using GitHub’s REST API to collect all of the PR’s changes and then keep those that are in Kotlin files and were added or modified. For that I used kscript and I was able to leverage all the libraries that I was accustomed to, like Retrofit and Moshi. When I was happy with the resulted script I used its --package option to create a standalone binary and copy it in the action’s Docker image.

The second part is a combination of bash commands that execute the ktlint binary by passing to it the results of the first part. Ktlint is being called with the --reporter=json parameter in order to create a JSON report.

The third and final part is again a kscript script that uses the report created before and GitHub’s REST API to make a PR line comment for every ktlint error that is part of the PR’s diff. Again a standalone binary was created and put in the image.

Note:

I like kscript since I can write things fast, easy and with all the libraries that I know but I also like writing test first and that proved to be quite difficult. So what I ended up doing was to act as if I was in a Kotlin project. I created my tests (using all my favorites like junit5, hamkrest and MockWebServer) and from that I created .kt files with the proper functionality. And for having a script I created a .kts file where I defined the external dependencies and included the .kt files:

import kotlin.system.exitProcess
//DEPS com.squareup.moshi:moshi:1.9.3
//DEPS com.squareup.moshi:moshi-kotlin:1.9.3
//DEPS com.squareup.retrofit2:retrofit:2.9.0
//DEPS com.squareup.retrofit2:converter-moshi:2.9.0
//INCLUDE logging.kt
//INCLUDE common.kt
//INCLUDE collectPrChanges.kt
//INCLUDE createGithubEvent.kt
val result = collectPrChanges(args)
if (result != 0) {
exitProcess(result)
}
println("Changes collected")
view raw exec-kscript.kts hosted with ❤ by GitHub
Action’s metadata

From the start what I wanted for this action was to be as autonomous as possible leaving very little responsibilities to the consumer and allowing her to just plug it in and watch it play.

For that the only input the action needs is a token, for allowing the kscript scripts to communicate with the API, which will most likely be the default secrets.GITHUB_TOKEN making the action’s usage as simple as adding the following lines in your workflow:

- uses: le0nidas/ktlint-pr-comments@v1
  with:
    repotoken: ${{ secrets.GITHUB_TOKEN }}
Docker image

A lot of things must happen in order to have the action ready to run. Sdkman, Kotlin and kscript must be installed, the code needs to be retrieved from the repository and both kscript scripts have to be packaged. On top of that ktlint must be downloaded and placed in the proper path.

For all that, and to shave a few seconds from the action, I decided to have an image that has everything ready. So I created a workflow that gets triggered every time there is a push in the main branch, builds and packs everything in an image and pushes the result to Docker Hub.

So now the action simply uses that image to run a container without any other ceremonies.

Note:

Before using Docker Hub I tried to use GitHub Packages but it turns out that public is not that public* since it requires an authentication to retrieve a package.

Summary

That’s it! An action for having ktlint’s report as comments in your PR. A result of trial and error since I wrote it while learning about actions but I hope that someone might find it useful. If you do let me know!

An example of how to use it can be found in the action’s repository where I dogfood it to the project.

* [3 Sep 2020]: looks like things will change