Pure functions, JavaScript and Unit Testing


Photo by Christopher Burns on Unsplash

I’ve started to write a small game on GitHub to try Node.js, because I’ve never touched Node.js during work hours. On my current project I’ve back-end written with Microsoft .NET. I’ve decided to not use any UI framework, I just added Browserify to share some code between front-end source code and back-end source code. I’ve written a small amount of code and tried to cover this code with Unit Tests. And at this point I found that I have nothing to cover…

Let me discus this statement a little bit, because I believe it sounds strange. Let’s take a look at the sample code (I’ve added short comments, so you can simply look through the comments to get the context. I’ll discuss each function in details a little bit later and you can return to the code sample):

'use strict';

// Some shared constants.
let constants = require('../../src-common/constants');
// Component responsible for field rendering.
let fieldView = require('./field-view');

const FieldSize = constants.FieldSize;
let FieldState = constants.FieldState;

let field;

// Creates game field, two-dimensional array.
// Default value based on one of the constants.
function resetField() {
field = [];

for (let i = 0; i < FieldSize; ++i) {
field.push([]);

for (let j = 0; j < FieldSize; ++j) {
field[i].push(FieldState.Empty);
}
}
}

// Custom (we are not using jQuery) event delegation.
// Calculated the position of the TD in the TABLE.
function getPosition(event) {
let result = null;

if (event.target.tagName === 'TD') {
let column = event.target;
let row = column.parentNode;
let indexOf = Array.prototype.indexOf;

result = {
row: indexOf.call(row.parentNode.children, row),
column: indexOf.call(row.children, column)
};
}

return result;
}

// Handles the user click and toggles the appropriate field.
function toggleShipDeck(event) {
let position = getPosition(event);

if (position) {
let r = position.row;
let c = position.column;

field[r][c] = field[r][c] === FieldState.Empty ?
FieldState.Deck :
FieldState.Empty;

fieldView.render(document.getElementById('client'), field);
}
}

// Subscription. No comments.
function listenToFieldConfiguration() {
let clientField = document.getElementById('client');

clientField.addEventListener('click', toggleShipDeck);
}

// "Public" API.
module.exports = {
startGame: () => {
listenToFieldConfiguration();
},
restart: () => {
resetField();
}
};

As you can see now I have only 2 functions in the public API and this API returns no value to the outer world, because… it’s… UI…

Now we are close enough to the two problems/myths of the UI development process:

  1. We should hide most of our code under closures if possible. Everything which is not called from the outside should be private. Private functions should not be tested during the Unit Testing process. This is internal behavior of the system and we should not really care about it;
  2. It’s fine to modify the state in place, to have private dependencies and side effects;

Private Functions should not be tested.

I would not argue with this statement basically. I agree with it anyway. But the issue is close enough to this statement. I believe every more or less skilled JavaScript developer can write code hiding most of the logic under closures, it’s not a big deal, and consider this code private. But unfortunately it’s just an illusion of private code. The trick is that the code is not truly private, we have dozens of callbacks, event handlers. User Interfaces are always based on Event-Driven Architecture. We are trying to hide event model into Reactive Programming, organize them using Redux, etc., because such architecture scales really hard. You are ending up with a spaghetti-code and recursive events (but it’s another topic). But we still have events underneath. And in my opinion we cannot treat event handlers as private behavior, because it’s not. It’s public API of our code, which called by the Browser itself (or by another object if we are talking about Event-emitter patterns).

So yes, in my opinion these 2 functions should be covered by the Unit Tests, because they are public:

  • toggleShipDeck – event handler, hopefully after my explanation it’s more or less clear;
  • getPosition – this one is a little bit trickier. This is basically a kind of helper function, we implement a common logic of event delegation here. Probably it would be a good idea to generalize it and move to project’s framework;

Side effects and dependencies
Statement: It’s fine to modify the state in place, to have private dependencies and side effects.

Unfortunately it’s the way it’s done in millions of production applications. It’s an old good way of doing things. We receive an event, handle it based on our business logic and modify the internal (yeah, again private) state of the application, syncing data with server, render new DOM. Basically it work but… Let’s think about issues we face when try to test resetField function:

  1. Hidden dependencies. For example we know that at some point customer will need to be able to customize field size. And we want right now check that our code works fine with different field sizes. We want to make sure, that we can use different default cell value. But how to do it? Constants extracted on the bootstrapping phase and we have no ability to replace them or mock in Unit Tests. The answer is injections. Every dependency should be injected from the outside. You need to think in a little bit different way, but it’s not so hard as it seems. If it’s some dependency which used throughout the module maybe we should create some constructor and inject it in this constructor. If it’s a local dependency – the obvious answer is parameter of the function.
  2. Field is modified just in place. We cannot check, that field was changed at all. We have no ability to check the previous state of the field, compare it with the new one… How should we modify the code to address this issue? The answer is to use pure functions. Think about each function in your code as of a -> b transformation. You have some data a and the result should always be the same when you bypass the same data as a parameter. It’s not so easy as far as UI applications always have dependencies, hidden state and all that stuff. But in my opinion it’s the most important, because it’s the only way we can create stable, testable code.

The last example.:

// Most of the code left unchanged.
function createField(size, defaultState) {
let newField = [];

for (let i = 0; i < size; ++i) {
newField.push([]);

for (let j = 0; j < size; ++j) {
newField[i].push(defaultState);
}
}

return newField;
}

// Most of the code left unchanged.
module.exports = {
startGame: () => {
listenToFieldConfiguration();
},
restart: () => {
let empty = constants.FieldState.Empty;
field = createField(constants.FieldSize, empty);
},
_toggleShipDeck: toggleShipDeck,
_getPosition: getPosition,
_createField: createField,
_cloneField: cloneField
};
  1. Every public function moved to export object. It’s not the super-elegant way, but at least we are now not trying to hide public functions into closures, so we can test them;
  2. createField now accepts 2 new parameters: size and default state. We have this values injected in restart function, but it’s fine, we still can test it by using _createField
  3. createField now returns new field object, instead of modifying it in place, so we can check that it creates what we expect;
Visualizing the changes

As a result of all these changes you’ll be able to write really simple Unit Tests, for example (this one is using Jasmine):

it('"createField" should create a field of appropriate size.',
() => {
const expectedFieldSize = 5;

let field = gameSetup._createField(expectedFieldSize);

expect(field.length).toBe(expectedFieldSize);
field.forEach(row => {
expect(row.length).toBe(expectedFieldSize);
});
});

Conslusion

A lot of these things are discussed widely in the Internet. But I believe, that a lot of developers still don’t think that it’s really important and it saves time for testing, debugging and bug fixing. So it’s not something new, but it’s still not a common practice. One of the possible solutions is to use Clojure or Elm languages instead of JavaScript itself. But there are some disadvantages of such approach: further developers fragmentation (JS-dev, Clojure-dev, Elm-dev, TypeScript-dev, etc.), it’s not simple to re-write existing projects, which are actively developed and have huge code base.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s