dimanche 15 avril 2018

Bug pattern - Hidden testable code

When there's a bug it is very likely the existing design contributed to it's existence. This suggests that there are corresponding design anti-patterns, patterns of code that are latent bugs

This time lets look at the pattern Hidden testable code

Hidden testable code

By Loveteamin [CC BY-SA 3.0 (https://creativecommons.org/licenses/by-sa/3.0)], from Wikimedia Commons

Problem: Easily testable code is hidden inside hard to test code. For this reason the code is either not tested or the tests are not as thourough as would be needed.

Schematically it looks like this
function hardToTest() {
let data = callToDependency()
//
// pure logic
// pure logic
// pure logic
//
let dataOther = callToDependency2(result)
}


Solution: It is very very cheap to fix this. Just an Extract Method automatic refactoring in order to extract a Pure Function that can be extremely easily tested.

function hardToTest() {
let data = callToDependency()
let result = easyToTestPureFunction(data)
let dataOther = callToDependency2(result)
}
function easyToTestPureFunction(data) {
//
// pure logic
// pure logic
// pure logic
//
}


Recently I came across an example of this bug generator pattern. It had a first bug that was fixed and tests weren't added because it was considered too costly to add tests for it. However the idea of testing an extracted function was not even considered. Two weeks later it became clear that this piece of code contained a second similar bug. The code had some very extensive transformation logic between two calls to a web service and two calls to the persistence layer. Below is a shortened version of it. The problems were both errors in the pure transformation logic.

function hardToTest() {
let data = requestExternalServer() // hard to test
let lang = persistence.get(data.id)) // hard to test
// below pure transformation logic
let langToUpdate = {};
versionsLangs.map((versionLang) => {
let restPath = versionLang.entity.toRestPath();
lang.dates.langsDates.map(datePayload => {
if (restPath === datePayload.langRestPath) {
langToUpdate[restPath] = langToUpdate[restPath] || {};
langToUpdate[restPath].dates = datePayload.payload.dates;
}
});
});
// hard to test again
return persistence.update(data.id, langToUpdate)
}


Once extracted and thoroughly tested the logic can be both refactored and extended with confidence. Bugs will have a much harder time hiding in this kind of light.


// This function needs only one test
function hardToTest() {
let data = requestExternalServer()
let lang = persistence.get(data.id))
let langToUpdate = makeLangToUpdate(lang)
return persistence.update(data.id, langToUpdate)
}
// This functionc can be thouroughly tested
function makeLangToUpdate(versionLangs, lang) {
let langToUpdate = {};
versionsLangs.map((versionLang) => {
let restPath = versionLang.entity.toRestPath();
lang.dates.langsDates.map(datePayload => {
if (restPath === datePayload.langRestPath) {
langToUpdate[restPath] = langToUpdate[restPath] || {};
langToUpdate[restPath].dates = datePayload.payload.dates;
}
});
});
}


The idea that we can cleanse the system from most of the bugs is called BugsZero. You can learn more about it here, and by various talks by Arlo Belshee, available on youtube.