11

After years of cowboy coding, I decided to pick up a book on how to write good quality code. I'm reading Clean Code by Robert Cecil Martin. In chapter 3 (functions) there is a section on dyadic functions. Here is an excerpt from the book.

Even obvious dyadic functions like assertEquals(expected, actual) are problematic. How many times have you put the actual where the expected should be? The two arguments have no natural ordering. The expected, actual ordering is a convention that requires practice to learn.

The author makes a compelling point. I work in machine learning and come across this all the time. For example, all of the metric functions in the sklearn library (probably the most used python library in the field) require you to be careful of the order of the inputs. As an example sklearn.metrics.homogeneity_score takes as inputs labels_true and labels_pred. What this function does isn't too relevant, what is relevant is that if you switch the order of the inputs no error will be thrown. In fact switching the inputs is equivalent to using another function in the library.

However the book does not go on to say a sensible fix for functions such as assertEquals. I cannot think of a fix for assertEquals or for functions I often come across like the one described above. What are good practices to solve this issue?

3 Answers 3

12

It is good to be aware of a possible issue even when there's no fix – that way you can be vigilant when reading or writing such code. In this specific example, you just get used to the order of arguments after a while.

There are language level ways to prevent any confusion about parameter order: named arguments. This is unfortunately not supported in many languages with C-style syntax such as Java or C++. But in Python, every argument can be a named argument. Instead of calling a function def foo(a, b) as foo(1, 2), we can do foo(a=1, b=2). Many modern languages like C# have similar syntax. The Smalltalk language family has taken named arguments furthest: there aren't any positional args and everything is named. This can lead to code that reads very close to natural language.

A more practical alternative is to create APIs that simulate named arguments. These can be fluent APIs, or helper functions that create a natural flow. The assertEquals(actual, expected) name is confusing. Some alternatives I have seen:

  • assertThat(actual, is(equalTo(expected))): by wrapping some arguments in helper types, the wrapping functions effectively serve as parameter names. In the specific case of unit test assertions, this technique is used by Hamcrest matchers to provide an extensible and composable assertion system. The disadvantage here is that you get a lot of nesting, and need to import lots of helper functions. This is my go-to technique in C++.

  • expect(actual).to.be(expected): a fluent API where you string function calls together. While this avoids extra nesting, this isn't very extensible. While I find that fluent APIs read very well, designing a good fluent API tends to take a lot of effort in my experience, because you need to implement additional classes for non-terminal states in the call chain. This effort only really pays off in the context of an autocompleting IDE that can suggest the next allowed method calls.

5

There are several methods to avoid this problem. One that doesn't force you to change the method you call:

Rather than

assertEquals( 42, meaningOfLife() ); 

Use

expected = 42;
actual = meaningOfLife();
assertEquals(expected, actual);

This forces the convention out into the open where it's easy to spot them getting switched. Sure it's not as easy to write but it is easy to read.

If you can change the method being called you can use the typing system to force usage that is easy to read.

assertThat( meaningOfLife(), is(42) );

Some languages let you avoid this because they have named parameters:

assertEquals( expected=42, actual=meaningOfLife() );

Others don't so you simulate them:

assertEquals().expected(42).actual( meaningOfLife() );

Whatever you do find a way that makes it obvious which is correct when read. Don't make me guess what the convention is. Show it to me.

0

I randomly happened upon this question and have definitely made this exact mistake in the past. I've come across two approaches to this problem that I particularly liked, each being slightly different than the ones already mentioned.

The first relies on formulating the assertions as regular boolean expressions, while the implementation retains an awareness of their structure. For example ASSERT(meaningOfLife() == 42), which could be reported as

Expected meaningOfLife() == 42, but got 0 == 42

This approach is taken by the C++ testing framework Catch2. It involves a lot of preprocessor voodoo and may not be possible in other languages.

The second is a fluent approach that splits the actual and expected values, similar to the suggestions in the other answers, but instead written simply as meaningOfLife().ShouldBe(42) and reported as

meaningOfLife() should be 42 but was 0

Although it's still possible to write this the wrong way around, that is not as likely because the phrasing naturally implies the meaning of "the actual value should be (equal to) the expected value".

This example is based on Shouldly, an assertion framework for .NET. The same approach could be used in other languages that support extending types in a similar fashion, such as Kotlin.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.