Every Unit Test Is a Stage Play – Part V

In this series about describing unit tests with the metaphor of a stage play that tells short stories about your system, we already published four parts:

Today, we look at the critics.

An integral part of the theater experience is the appraisal of the critics. A good review of a stage play can multiply the viewer count manyfold, while a bad review can make avid visitors hesitate or even omit the visit.

In our world of source code and unit tests, we can define the whole team of developers as critics. If they aren’t fond of the tests, they will neglect or even abandon them. Tests need to prove their worth in order to survive.

Let us think a little deeper about this aspect: Test code is evaluated more critically than any other source code! Normal production code can always claim to be wanted by the customer. No matter how bad the production code may look and feel like, it cannot just be deleted. Somebody would notice and complain.

Test code is not wanted by the customer. You can delete a test and it would not be noticed until a regression bug raises the question why the failing functionality wasn’t secured by a test. So in order to survive, test code needs a stakeholder inside the development team. Nobody outside the team cares about the test.

There is another difference between production code and test code: Production code is inherently silent during development. In contrast to this, test code is programmed to drive the developer’s attention to it in case of a crisis. It is code that tries to steal your focus and cries wolf. It is the messenger that delivers the bad news.

Test code is the code you’ll likely read in a state of irritation or annoyance.

Think about a theater critic that visits and rates a stage play in a state of irritation and annoyance. That wanted to do something else instead and probably has a deadline to meet for that other thing. His opinion is probably biased towards a scathing critique.

We talked about several things that test code can do to be inviting, concise, comprehendible and plausible. What it can’t do is to be entertaining. Test code is inherently boring. Every test is a short story that seems trivial when seen in isolation. We can probably anticipate the critique about such a play: “it meant well, but was ultimately forgettable”.

What can we do to make test code more meaningful? To convey its impact and significance to the critics?

In the world of theater (and even more so: movies), one strategy is to add “big names” to the production: “From the director of Known Masterpiece” or “Part III of the Successful Series”.

Another strategy is to embellish oneself with other critiques (hopefully good ones): “Nominated for X awards” or “Praised by Grumpy Critic”.

Let’s translate these two strategies into the world of unit tests:

Strategy 1: Borrow a stakeholder by linking to the requirement

I stated above that test code has no direct stakeholder. That’s correct for the code itself, but not for its motivation to exist. We don’t write unit tests just to have them. We write them because we want to assert that some functionality is present or some bug is absent. In both cases, we probably have a ticket that describes the required change in depth. We can add the “big name” of the ticket to the test by adding its number or a full url as a comment to the test:

/**
 * #Requirement http://issuetracker/TICKET-3096
 */
@Test
public void understands_iso8601_timestamp() {
    final LocalDateTime actual = SomeController.dateTimeFrom(
        "2023-05-24T17:30:20"
    );
    assertThat(
        actual
    ).isEqualTo(
        "2023-05-24T17:30:20"
    );
}

The detail of interest is the comment above the test method. It explains the motivation behind authoring the test. The first word (#Requirement) indicates that this is a new feature that got commissioned by the customer. If it was a bugfix test instead, the first word would be #Bugfix. In both cases, we tell future developers that this test has a meaning in the context of the linked ticket. It isn’t some random test that annoys them, it is the guard for a specific use case of the system.

Strategy 2: Gather visible awards for previous achievements

Once you get used to the accompanying comment to a test method, you can see it as some kind of billboard that displays the merit of the test. Why not display the heroic deeds of the test, too? I’ve blogged about the idea a decade ago, so this is just a quick recap:

/**
 * #Requirement http://issuetracker/TICKET-3096
 * @lifesaver by dsl
 * @regression by xyz
 */
@Test
public void understands_iso8601_timestamp() {
    /* omitted test code */
}

Every time a test does something positive for you, give it a medal! You can add it right below the ticket link and document for everybody to see that this test has earned its place in the code base. Of course, you can also document your frustrating encounters with a specific test in the same way. Over time, the bad tests will exhibit several negative awards, while your best tests will have several lifesaver medals (the highest distinction a test can achieve).

So, to wrap up this part of the metaphor: Pacify the inevitable critics of your test code by not only giving them pleasant code to look at but also context information about why this code exists and why they should listen to it if it happens to have grabbed their attention, even with bad news.

Epilogue

This is the fifth part of a series. All parts are linked below:

Why Java’s built-in hash functions are unsuitable for password hashing

Passwords are one of the most sensitive pieces of information handled by applications. Hashing them before storage ensures they remain protected, even if the database is compromised. However, not all hashing algorithms are designed for password security. Java’s built-in hashing mechanisms used e.g. by HashMap, are optimized for performance—not security.

In this post, we will explore the differences between general-purpose and cryptographic hash functions and explain why the latter should always be used for passwords.

Java’s built-in hashing algorithms

Java provides a hashCode() method for most objects, including strings, which is commonly used in data structures like HashMap and HashSet. For instance, the hashCode() implementation for String uses a simple algorithm:

public int hashCode() {
    int h = 0;
    for (int i = 0; i < value.length; i++) {
        h = 31 * h + value[i];
    }
    return h;
}

This method calculates a 32-bit integer hash by combining each character in the string with the multiplier 31. The goal is to produce hash values for efficient lookups.

This simplicity makes hashCode() extremely efficient for its primary use case—managing hash-based collections. Its deterministic nature ensures that identical inputs always produce the same hash, which is essential for consistent object comparisons. Additionally, it provides decent distribution across hash table buckets, minimizing performance bottlenecks caused by collisions.

However, the same features that make the functions ideal for collections are also its greatest weaknesses when applied to password security. Because it’s fast, an attacker could quickly compute the hash for any potential password and compare it to a leaked hash. Furthermore, it’s 32-bit output space is too small for secure applications and lead to frequent collisions. For example:

System.out.println("Aa".hashCode()); // 2112
System.out.println("BB".hashCode()); // 2112

The lack of randomness (such as salting) and security-focused features make hashCode() entirely unsuitable for protecting passwords. You can manually add a random value before passing the string into the hash algorithm, but the small output space and high speed still make it possible to generate a lookup table quickly. It was never designed to handle adversarial scenarios like brute-force attacks, where attackers attempt billions of guesses per second.

Cryptographic hash algorithms

Cryptographic hash functions serve a completely different purpose. They are designed to provide security in the face of adversarial attacks, ensuring that data integrity and confidentiality are maintained. Examples include general-purpose cryptographic hashes like SHA-256 and password-specific algorithms like bcrypt, PBKDF2, and Argon2.

They produce fixed-length outputs (e.g., 256 bits for SHA-256) and are engineered to be computationally infeasible to reverse. This makes them ideal for securing passwords and other sensitive data. In addition, some cryptographic password-hashing libraries, such as bcrypt, incorporate salting automatically—a technique where a random value is added to the password before hashing. This ensures that even identical passwords produce different hash values, thwarting attacks that rely on precomputed hashes (rainbow tables).

Another critical feature is key stretching, where the hashing process is deliberately slowed down by performing many iterations. For example, bcrypt and PBKDF2 allow developers to configure the number of iterations, making brute-force attacks significantly more expensive in terms of time and computational resources.

Conclusion

Java’s built-in hash functions, such as hashCode(), are designed for speed, efficiency, and consistent behavior in hash-based collections. They are fast, deterministic, and effective at spreading values evenly across buckets.

On the other hand, cryptographic hash algorithms are purpose-built for security. They prioritize irreversibility, randomness, and computational cost, all of which are essential for protecting passwords against modern attack vectors.

Java’s hashCode() is an excellent tool for managing hash-based collections, but it was never intended for the high-stakes realm of password security.

A few more heuristics for rejecting Merges

Since a few weeks ago, I am trying to find a few easy things to look for when facing a Merge Request (also called “Pull Request”) that is too large to be quickly accepted.

When facing a larger Merge Request, how can one rather quickly decide whether it is worth going through all changes in one session, or to decide that this is too dangerous and reject.

These thoughts apply for a medium-sized repository – I am of the opinion that if you happen to work in a large project, or contribute to a public open-source repository, one should never even aim for larger merge requests, i.e. they should be rejected if there is more than one reason any code changed in that MR.

Being too strict just for the sake of it, in my eyes, can be a costly mistake – You waste your time in unnecessary structure and, in earlier / more experimental development stages, you might not want to take the drive out of a project. Nevertheless, maintainers need to know what’s going on.

Last time, I kept two main thoughts open, and I want to discuss these here, especially since they now had time to flourish a while in the tasty marinade that is my brain.

Can you describe the changes in one sentence?

I want my code to change for a multitude of reasons, but I want to know which kind of “glasses” I read these changes with. For me, it is a lesser problem to go through many changes if I can assign them to the same “why”. I.e. introducting i18n might change many lines of codes, but as these happen for the same reason, they can be understood easily.

But if, for some reason, people decide to change the formatting (replace tabs with spaces or such shenanigans), you better make sure that this the only reason any line changes. If there is any other thing someone did “as it just appeared easy” -reject the whole MR. That is no place for the “boy scout rule”, it is just too dangerous.

For me, it is too little to always apply the same type of glasses to any Merge Request there is. One could say “I only look for technical correctness”. But usually I can very well allow myself some flexibility there. I need to know, however, that all changes happened for only a few given reasons, because only then I can be sure that the developer did not actually loose track of his goal somewhere on the way.

Does this Merge increase the trust in a collaboration?

From a bird-eye point of view, people working together should always pay attention whether a given trajectory goes in the direction of increasing trust. Of course, if you fix a broken menu button in a user interface of a large project, the MR should just do that – but if you are in a smaller project with the intention of staying there, I suggest that every MR expresses exactly that: “I understand what is important at the current stage of this collaboration and do exactly that”.

Especially when working together for a longer time, it can be easy to let the branching discipline slip a little – things might have gone well for a longer time. But this is a fragile state, because if you then care too little about the boundaries of a specific MR, this can damage the trust all too easily.

In a customer project, this trust goes out to the customer. This might be the difference between “if something breaks they’ll write you a mail, you fix it, they are happy” and “they insist on a certain test coverage”.

Conclusion

So basically, reviewing the code of others boils down to writing own code, or improving User Experience, or managing anything – think not in a list of small-scale checklists, think in terms of “Cognitive Load”. A good programmer should have a large set of possible glasses (mindsets) through which they see code, especially foreign code. One should always be honest whether a given change is compatible with only a very small number of reasons. If there is a Merge Request that allows itself to do too much, this is not the Boy Scout Rule – it is a recipe of undermining mutual trust. Do not overestimate your own brain capacity. Reject the thing, and reserve that capacity for something useful.

Heterogeneous lookup in unordered C++ containers

I often use std::string_view via the sv suffix for string constants in my code. If I need to associate something with those constants at runtime, I put it in an std::unordered_map with the constants as the keys.

Just a few days ago, I was using and std::unordered_map<std::string, ...> and wanted to .find(...) something in it with such a string constant. But that didn’t compile. From long ago, I remember that the type must be identical, and since there is no implicit conversion from std::string_view to std::string, I made that explicit to get it to compile. But wait. Didn’t C++ add support for using a different type than the key_type for the lookup? Indeed it did, in P0919R3 and P1690R1 from last decade. All major compilers seem to support it too. Then why wasn’t this working? It turns out that it’s not enabled by default, you need to explicitly enable it by supplying a special hasher. Here’s how I do it:

struct stringly_hash
{
  using is_transparent = void;
  [[nodiscard]] size_t operator()(char const* rhs) const
  {
    return std::hash<std::string_view>{}(rhs);
  }
  [[nodiscard]] size_t operator()(std::string_view rhs) const
  {
    return std::hash<std::string_view>{}(rhs);
  }
  [[nodiscard]] size_t operator()(std::string const& rhs) const
  {
    return std::hash<std::string>{}(rhs);
  }
};

template <typename ValueType>
using unordered_string_map = std::unordered_map<
  std::string,
  ValueType,
  stringly_hash,
  std::equal_to<>
>;

This is almost the same code as the sample given in the first of the two proposals. The using is_transparent = void; is how the feature is enabled and was changed in the second proposal.

JSON as a table in PostgreSQL 17

Some time ago, I described in a blog post how to work with JSON data in a PostgreSQL database. Last month (September 2024), PostgreSQL 17 was released, which offers another feature for working with JSON data: the JSON_TABLE() function. Such a function already existed in other database systems, such as MySQL and Oracle.

The core idea behind JSON_TABLE() is to temporarily transform JSON data into a relational format that can be queried using standard SQL commands. In doing so, developers can apply the full power of SQL, such as filtering, sorting, aggregating, and joining, to data that originally exists in JSON format: It enables you to use SQL to query JSON data stored in a column as though it were relational data, and you can join JSON data with regular relational tables for a seamless mix of structured and semi-structured data.

Syntax

The syntax for JSON_TABLE looks a bit complex at first glance, but once you break it down, it becomes quite clear. Here’s the basic structure:

JSON_TABLE(
  json_doc, path
  COLUMNS (
    column_name type PATH 'path_to_value' [DEFAULT default_value ON ERROR],
    ...
  )
) AS alias_name

The json_doc is the JSON data you’re querying. It can be a JSON column or a JSON string. It is followed by a path expression, which describes the location in the JSON document that you want to process. Path expressions are specified as strings in single quotation marks and have a special syntax called JSONPath (loosely inspired by XPath for XML). For example, such an expression could look like this: '$.store.book[0].title'. The dollar sign represents the root of the JSON tree, the sub-properties are separated by dots, and array elements are referenced using square brackets.

This is followed by the COLUMNS keyword. It specifies the columns to be extracted from the JSON document. Each column is specified by a name of the column in the resulting table, a data type of the extracted value (e.g., VARCHAR, INT), followed by the PATH keyword and a JSON path expression that references a JSON value below the original path expression.

The DEFAULT keyword optionally provides a default value if the path does not exist or if there’s an error during extraction.

After the JSON_TABLE function call you can specify the alias name for the table with the AS keyword.

Example

It’s time for an example. Let’s assume we have a table named fruit_store with a column named json_details that holds the some JSON data about fruits:

INSERT INTO fruit_store (id, json_details) VALUES (
    1,
    '{
        "category": "Citrus",
        "fruits": [
            {
                "name": "Orange",
                "color": "Orange",
                "taste": "Sweet",
                "price_per_kg": 3.5
            },
            {
                "name": "Lemon",
                "color": "Yellow",
                "taste": "Sour",
                "price_per_kg": 4.0
            }
        ]
    }'
);

Now we can use the JSON_TABLE function to extract the details of each fruit from the JSON document. The goal is to retrieve the fruit name, color, taste, and price for each fruit in the array. Here’s the query:

SELECT *
  FROM fruit_store,
  JSON_TABLE(
    json_details, '$.fruits[*]'
    COLUMNS (
      fruit_name   VARCHAR(50)  PATH '$.name',
      fruit_color  VARCHAR(50)  PATH '$.color',
      fruit_taste  VARCHAR(50)  PATH '$.taste',
      price_per_kg DECIMAL(5,2) PATH '$.price_per_kg'
    )
  ) AS fruit_table;

Running this query will give you the following result:

idfruit_namefruit_colorfruit_tasteprice_per_kg
1OrangeOrangeSweet3.50
1LemonYellowSour4.00

Git revert may not be what you want!

Git is a powerful version control system (VCS) for tracking changes in a source code base as most developers will know by now… It is also known for its quirky command line interface and previously less known concepts like staging area, cherry-picking and rebasing.

This can make Git quite intimidating and there are many memes about how confusing and hard it is to work with.

Especially for people coming from Subversion (SVN) there are a few changes and pitfalls because some commands have the same name but either do different things in the context or altogether, e.g. commit, checkout and revert.

Commit in the SVN world perform synchronization with the remote repository whereas a commit in git is local and has to synchronized with the remote repository using push. Checkout is similar and maybe even worse: In SVN it fetches the remote repository state and puts it in your working copy. In Git is used to replace files in your working copy with contents from the local repository or formerly to switch and/or create local branches…

But now on to our main topic:

What does git revert actually do?

Revert is maybe the most misleading command existing in both SVN and Git.

In SVN it is quite simple: Undo all local edits (see svnbook). It throws away you uncommited changes and causes potential data loss.

Git’s revert works in a completely different way! It creates “inverse” commit(s) in your (local) repository. Your working copy has to be clean without changes to be able to revert and you will undo the specified commit and record this change in the repository and its history.

So both revert commands are fundamentally different. This can lead to unexpected behaviour, especially when reverting a merge commit:

We had the case where our developers had two feature branches and one dev decided to temporarily merge the other feature branch to test some things out. Then he reverted the merge using git revert and opened a merge request. This leads to the situation, that the other feature branch becomes unmergeable because git records no changes. This is rightfully so, because this was already merged as part of the first one, but without the changes (because they were reverted!). So all the changes of the other feature branch were lost and not easily mergeable:

To fix a situation like this you can cherry-pick the reverted commit mentioned in the commit message.

How to revert local changes in git

So revert is maybe not what we wanted to do in the first place to undo our temporary merge of the other feature branch. Instead we should use git reset. It has some variants, but the equivalent of SVN’s revert would be the lossy

git reset --hard HEAD

to clean our working copy. If we wanted to “revert” the merge commit in our example we would do

git reset --hard HEAD^

to undo the last commit.

I hope this clear up some confusion regarding the function of some Git commands vs. their SVN counterparts or “false friends”.

Every Unit Test Is a Stage Play – Part IV

In this series about describing unit tests with the metaphor of a stage play that tells short stories about your system, we already published three parts:

Today, we look at the story.

When you visit a theater, you probably expect to be entertained. You expect some level of preparation and presentation. You might not enjoy every aspect of the stage play, but you can cherish the overall experience.

When you read an unit test as a developer, you should not expect to be entertained. But you can expect some level of presentation and you should be able to endure the overall experience.

In both cases, a great factor to success is how the story is presented to you.

Imagine trying to follow a stage play that is in rehearsal mode. Constant interruptions and corrections from outside the stage, repetitions of scenes and single sentences and sometimes omissions that everybody is clued in on except you. And of course, nobody is dressed for their role. It would be hard to follow the plot and piece the story together.

Unit test code often reads like an early rehearsal. The code is stitched together by copy & paste, some details are modified but not emphasized and the point of the story is only revealed at the end, oftentimes told indirectly by convoluted assertions. When the test runs green for the first time, it is abandoned and left as an exercise in improvement for the next reader.

The next reader is a developer that made a change to the production code that got red-flagged by the unit test. He or she tries to find out why the jury of assertions is against the change and what the test is all about. It’s like the first visitor of a stage play has to tell the lighting technician where to point the spotlights without knowing how the story will play out.

If we accept the metaphor and view unit tests as stage plays that tell a short story, we should try to tell the story in a clear and concise manner. Giving standard names to the participating roles is an important first step to clue in the visitor/reader. But the last part of a story is the most crucial one. You are expected to tie the story threads together and provide a resolution that can be followed.

In unit testing, we express the resolution of the unit test’s short story as assertions:

public void parsingOfErroneousODLState() {
    final SerialODL target = new SerialODL(Z, "19");
    
    final ODLState actual = target.getCurrentState();
    
    Assert.assertFalse(
        actual.isNormalOperation()
    );
    Assert.assertEquals(
        3,
        IterableUtil.getSizeFor(actual.getErrorStates())
    );
    Assert.assertEquals(
        ODLErrorState.TEST_ERROR,
        IterableUtil.getElementAt(0, actual.getErrorStates())
    );
    Assert.assertEquals(
        ODLErrorState.INVALID_VALUE_DUE_TO_INITIALIZING,
        IterableUtil.getElementAt(1, actual.getErrorStates())
    );
    Assert.assertEquals(
        ODLErrorState.VALUE_GREATER_ALARM_THRESHOLD,
        IterableUtil.getElementAt(2, actual.getErrorStates())
    );
}

This unit test consists of one line of preparation (“arrange”), one line of code under test that produces the “actual” (“act”) and five assertions on several lines each (“assert”). Nearly 80 percent of this unit test are assertions. And they try to express something, but it gets drowned in noise.

One key to a better story is the usage of a more fitting form of expression, in our case a more natural way to write assertions:

public void parsingOfErroneousODLState() {
    final SerialODL target = new SerialODL(Z, "19");

    final ODLState actual = target.getCurrentState();

    assertThat(
        actual.isNormalOperation()
    ).isFalse();

    assertThat(
        actual.getErrorStates()
    ).containsExactly(
        ODLErrorState.TEST_ERROR,
        ODLErrorState.INVALID_VALUE_DUE_TO_INITIALIZING,
        ODLErrorState.VALUE_GREATER_ALARM_THRESHOLD
    );
}

In this example, we used assertj fluent assertions. As you can see, you can shrink the assertions part of your story down to the essence. You can state what you really want to see and not hide it behind indices and size comparisons that only exist because of the indices.

Another way to guide your reader is by structuring your test story into a standard form. From classic storytelling, we know about the hero’s journey that consists of three sections (departure, initiation, return) and can be found in countless books, movies and stage plays.

Our test’s journey is called AAA pattern. The three sections are:

  • Arrange
  • Act
  • Assert

Whenever you write an unit test, adhere to this pattern. If you find yourself tempted to add a second act or more assertions, break up your one unit test into two. You might want to think about extracting the arrange part into a common utility method (that is placed down below, behind the curtain). The story then says: The hero is in the same position both times, decides different (the two act sections) and has a different outcome (the two assertions) because of that.

There are probably countless things more that you can think of to make the story of your tests more compelling. Remember that test are not required to be entertaining or surprising. You can tell the same classic tale over and over again. The computer doesn’t mind and the next reader is glad when the test code is accessible right away because the structure and phrasing is on point.

Nobody would pay to see a confusing stage play. And nobody wants to decipher extravagant test code that just broke in an unexpected way. Give your readers what they hope for: Plain short stories about your system.

Epilogue

This is the fourth part of a series. All parts are linked below:

Every Unit Test Is a Stage Play – Part III

In the first and second parts of this series, I talked about describing unit tests with the metaphor of a stage play that tells short stories about your system.

This metaphor is really useful in many aspects of unit testing, as we have seen with naming variables and clearing the test method. In each blog entry of the series, I’m focussing on one aspect of the whole.

Today, we look at the theater.

If you go to the theater as a guest, you are greeted by a pompous entrance with a luxurious stairway that lead you to your comfy seat. You don’t get to see all the people and things behind the heavy curtains. You don’t need to recognize any details of the floor, the walls or the ceiling as you make your way into the auditorium. They don’t matter for the play.

If you enter the theater as an actor or a stage help, you slip into the building by the back entrance and make your way through a series of storage rooms. Or at least that is the cliché in many movies (I’m thinking about Birdman, but you probably have your own mental image at hands). You need to recognize all the details and position yourself according to your job. Your preparation matters for the play.

I described the test methods as single stage plays in earlier blog posts. Today, I want you to think about a test class as a theater. We need to agree on the position of the entrance. In my opinion, the entrance is where I’m starting to read – at the top of the file.

In my opinion, as a reader of the test class, I’m one of the guests. My expectation is that the test class is designed to be inviting to guests.

This expectation comes from a fundamental difference between production code classes and test classes: Classes in production code are not meant to be read. In fact, if you tailor your modules right and design the interface of a class clearly and without surprises, I want to utilize your class, but don’t read it. I don’t have to read it because the interface told me everything I needed to know about your class. Spare me the details, I’ve got problems to solve!

Test classes, on the other hand, are meant to be read. Nobody will call a test method from the production code. The interface of a test class is confusing from the outside. To value a test class is to read and understand it.

Production code classes are like goverment agencies: They serve you at the front, but don’t want you to snoop around the internals. Test classes are like a theater: You are invited to come inside and marvel at the show.

So we should design our test classes like theaters: An inviting upper part for the guests and a pragmatic lower part for the stage hands behind the curtain.

Let’s look at an example:

public class UninvitingTest {
	
    public static class TestResult {
        private final PulseCount[] counts;
        private final byte[] bytes;

        public TestResult(
            final PulseCount count,
            final byte[] bytes
        ) {
            this(
                new PulseCount[] {
                    count
                },
                bytes
            );
        }

        public TestResult(	
    	    final PulseCount[] counts,
            final byte[] bytes
        ) {
            super();
            this.counts = counts;
            this.bytes = bytes;
        }

        public PulseCount[] getCounts() {
            return this.counts;
        }

        public byte[] getBytes() {
            return this.bytes;
        }
    }
	
    private static final TestResult ZERO_COUNT = 
	new TestResult(
            new PulseCount(0),
            new byte[] {0x0, 0x0, 0x0, 0x0}
        );
    private static final TestResult VERY_SMALL_COUNT = 
	new TestResult(
            new PulseCount(34),
            new byte[] {0x0, 0x0, 0x22, 0x0}
        );
    private static final TestResult MEDIUM_COUNT_BORDER = 
	new TestResult(
            new PulseCount(65536),
            new byte[] {0x1, 0x0, 0x0, 0x0}
        );
    
    public UninvitingTest() {
    	super();
    }

    @Test
    public void serializeSingleChannelValues() throws Exception {
        SPEChannelValuesSerializer scv = 
            new SPEChannelValuesSerializer();
        Assertion.assertArrayEquals(
            ZERO_COUNT.getBytes(),
            scv.serializeCounts(ZERO_COUNT.getCounts())
        );
        Assertion.assertArrayEquals(
    	    VERY_SMALL_COUNT.getBytes(),
            scv.serializeCounts(VERY_SMALL_COUNT.getCounts())
        );
        Assertion.assertArrayEquals(
    	    MEDIUM_COUNT_BORDER.getBytes(),
            scv.serializeCounts(MEDIUM_COUNT_BORDER.getCounts())
        );
    }
}

In fact, I hope you didn’t read the whole thing. There are lots of problems with this test, but let’s focus on the entrance part:

  • Package declaration (omitted here)
  • Import statements (omitted here)
  • Class declaration
  • Inner class definition
  • Constant definitions
  • Constructor
  • Test method

The amount depends on the programming language, but some ornaments at the top of a file are probably required and can’t be skipped or moved around. We can think of them as a parking lot that we require, but don’t find visually appealing.

The class declaration is something like an entrance door. Behind it, the theater begins. And just by looking at the next three things, I can tell that I took the wrong door. Why am I burdened with the implementation details of a whole other class? Do I need to remember any of that? Are the constants important? Why does a test class require a constructor?

In this test class, I need to travel 50 lines of code before I reach the first test method. Translated into our metaphor, this would be equivalent to three storage rooms filled with random stuff that I need to traverse before I can sit into my chair to watch the play. It would be ridiculous when encountered in real life.

The solution isn’t that hard: Store your stuff in the back rooms. We just need to move our test method up, right under the class declaration. Everything else is defined at the bottom of our class, after the test methods.

This is a clear violation of the Java code conventions and the usual structure of a class. Just remember this: The code conventions and structures apply to production code and are probably useful for it. But we have other requirements for our test classes. We don’t need to know about the intrinsic details of that inner class because it will only be used in a few test methods. The constants aren’t public and won’t just change. The only call to our constructor lies outside of our code in the test framework. We don’t need it at all and should remove it.

If you view your test class as a theater, you store your stuff in the back and present an inviting front to your readers. You know why they visit you: They want to read the tests, so show them the tests as proximate as possible. Let the compiler travel your code, not your readers.

And just so show you the effect, here is the nasty test class from above, with the more inviting structure:

public class UninvitingTest {
    
    @Test
    public void serializeSingleChannelValues() throws Exception {
        SPEChannelValuesSerializer scv = 
            new SPEChannelValuesSerializer();
        Assertion.assertArrayEquals(
            ZERO_COUNT.getBytes(),
            scv.serializeCounts(ZERO_COUNT.getCounts())
        );
        Assertion.assertArrayEquals(
            VERY_SMALL_COUNT.getBytes(),
            scv.serializeCounts(VERY_SMALL_COUNT.getCounts())
        );
        Assertion.assertArrayEquals(
            MEDIUM_COUNT_BORDER.getBytes(),
            scv.serializeCounts(MEDIUM_COUNT_BORDER.getCounts())
        );
    }

    private static final TestResult ZERO_COUNT = 
        new TestResult(
            new PulseCount(0),
            new byte[] {0x0, 0x0, 0x0, 0x0}
        );
    private static final TestResult VERY_SMALL_COUNT = 
        new TestResult(
            new PulseCount(34),
            new byte[] {0x0, 0x0, 0x22, 0x0}
        );
    private static final TestResult MEDIUM_COUNT_BORDER = 
        new TestResult(
            new PulseCount(65536),
            new byte[] {0x1, 0x0, 0x0, 0x0}
        );

    public static class TestResult {
        private final PulseCount[] counts;
        private final byte[] bytes;

        public TestResult(
            final PulseCount count,
            final byte[] bytes
        ) {
            this(
                new PulseCount[] {
                    count
                },
                bytes
            );
        }

        public TestResult(  
            final PulseCount[] counts,
            final byte[] bytes
        ) {
            super();
            this.counts = counts;
            this.bytes = bytes;
        }

        public PulseCount[] getCounts() {
            return this.counts;
        }

        public byte[] getBytes() {
            return this.bytes;
        }
    }

    public UninvitingTest() {
        super();
    }
}

Show your readers the test methods and don’t burden them with details they just don’t need (yet).

Epilogue

This is the third part of a series. All parts are linked below:

A few heuristics for rejecting Merges

The title of this post is somewhat of a working title. It is based on the observation, that in larger projects a day might come where you have to outsource some amount of work (an “issue”, if you will) to anyone who is not you, and maybe not even someone you already know well. Or at least now, how well they fit your image of good code.

That concept is probably not new* for you, and neither is the idea that sooner or later, the foreign code has to be merged, and you are the reviewer. (depending on your version control system, the lingo might differ, but let’s call this whole process a Merge Request for now, like GitLab does).

Now, every issue is different, therefore it is not upon me to give you hard rules what a Merge Request should do. These rules would be as diverse as the issues themselves, and there is no clear answer to “as a reviewer, what should I actually look for?”.

The Best-Case Scenario (forget about it)

The best-case scenario might be:

  • You understand the issue well enough
  • You understand the code base well enough
  • You understand the language well enough
  • The changes are few enough that you can understand them quickly enough.

In that case, you do not need some blog post to help you.

So you could try and pull up some rules for your project in which every Issue leads to Best-Case Merge Requests, but as you will fail anyway, it might be the wiser thing to lower your expectation. I mean, if all issues would match that case, you probably would be faster by writing all the code yourself.

Rather go for the Not-Good-Enough Scenarios

So, what do you do to combine the requirements a) the collaboration should really simplify your life and b) you do not want to compromise your standards too heavily?

This thought process is not finished yet, but at least I would go for some heuristics – if these are violated too heavily, chances are that everyone involved might actually profit from having this Merge Request rejected.

  • Can the issue be grasped within a few minutes?
  • Can the changes be grasped in about half an hour or less?
  • Can the problematic code pieces be described in a few short points?
  • Does this increase the feeling of trust in the collaboration?
Can the issue be grasped within a few minutes?

If it can’t, or you can’t, how come you are the reviewer? This is probably the easiest heuristic to ignore, because real-life problems and real-life programmers might be so imperfect that this is utopian, but still, if it happens, be extra wary.

Can the changes be grasped in about half an hour or less?

The half-an-hour is only a rough figure, of course, but that’s not the point.

If you ever thought “my brain is great”, remember that this was your brain speaking. Your attention span is just not good. If you think otherwise, how come you are the reviewer?

It is easy to think that longer Merge Requests just take a longer time to process. But it doesn’t scale. You will use up your willpower, motivation, attention span quicker than you would like to admit; and if you then go for “well, at least I want to finish the thing now, otherwise I have to do it even again” you will probably let too many mistakes slip.

It is just not responsible. If you want to show willpower, go for increasing your willpower in admitting that some Merge Requests are just too big, and reject them.

One can always backup a branch, create a new one, cherry-pick commits on them, even only commit portions of the same file – this might feel like a punishment at first, but chances are that this process might actually find several problems that were hidden before.

To be continued…

I spoilered my two other heuristics above, but figure that this post is long enough already. Even if you don’t agree with e.g. the time spans given above – the main idea is: Do not let a large Merge Request through just because you believe that you are strong enough to handle it. There’s noone to be impressed.

The impressive part of good work is that it doesn’t look impressive.

I will continue this, and – maybe you have some ideas from your experience? What would you do when a Merge Request is too complex?

I have changed my stance on “using” in C++ headers

I used to be pretty strictly against using either C++ using-directives or -declarations from within header files. It kind of stuck with me as a no-go. But that has changed in recent years.

There are now good cases where using can go into a header. For example, I do not really like putting things like…

using namespace std::string_literals;
using namespace std::string_view_literals;
using namespace std::chrono_literals;

…at the beginning of each source file. Did you know that you can pull all those (and some more) in with a single using namespace std::literals? Either way, in my newer projects, these usually go into one of the more prominent headers. Same goes for other literal operators such as those from the SI library. And so do using declarations for common vocabulary types. E.g. 2D or 3D vector types , in math heavy projects. Of course, they always go after the specific #include(s) the using is referencing. The benefits of doing that usually outweigh the danger of name-clashes and weird order dependencies.

There are cases where I still avoid using in headers however, and that is when the given header is ‘public’, i.e. being consumed by something that is not under my organization’s control. In that case, you better leave that decision to the library consumer.