0

I am refactoring some code where a globally defined list is updated based on messages from an external system. Currently there is a global instance of the list. Various messages update the list based on domain specific rules.

I would like to write a unit test which can test the mechanism of sending a message and checking that the list is updated correctly.

I can change the code however I see fit. Ideally do this incrementally however because program in test environment needs to continue to work while I refactor.

An idea I had was to write a function which is used to update the list. The function acts on any list that is passed as a parameter.

This way a unit test can use its own list and check that eg message X results in the correct updating of the list.

Is this a reasonable approach? any other alternatives?

The specific environment is node.js. But probably applies to any language or environment.

  • 1
    Your idea of passing the list as a parameter is a good way to approach this. – BobDalgleish May 3 '18 at 14:32
3

Exactly, transforming global state into objects that you pass as parameters is the standard way to make a function testable.

So from this:

var globalList = [];

function doSomething() {
  ...
}

We would go to:

var globalList = [];

function doSomething() {
  return doSomethingTestable(globalList);
}

function doSomethingTestable(list) {
  ...
}

We have then introduced a seam into a design where we can inject test data and review the results.

By keeping the old function around (as a wrapper around the new, testable function), we do not have to update any dependents of this function immediately. So this refactoring is restricted to that function, and is therefore low-risk. In the future (when we have better unit tests), we can switch over the other parts to use the parameterized function. Eventually, the code is reduced to:

function doSomethingTestable(list) {
  ...
}

(A note on naming: the old function and the new function essentially want the same name. I don't actually recommend a ...Testable or ...improved suffix to disambiguate them! Sometimes this can be solved by not adding another function, and just a parameter with a default value. Sometimes the original name was bad, and this is a welcome opportunity for renaming. If none of the easy cases apply, I've found it best to describe the extra parameter, e.g. sendMessage() could become sendMessageToQueue(queue).)

Instead of passing the naked data as a parameter, it is often better to consider what kind of operations are performed on this data, and encapsulating those operations. We would then pass an object, where the methods on this objects document certain intentions of our code. For example, we might not need all operations for a list (such as indexing and iteration) and only want to push() and pop() elements.

Offering a restricted set of methods with self-documenting names tends to make the code more obvious and robust. It also makes the code easier to test, since we don't have to validate a specific result state, and can validate the actions of that code instead. (Though often, validating the complete output state is still the easiest solution.)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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