How fix to bugs before they happen

Take a look at the below function that searches an array and returns the value if found either as is or as the result of a callback function:

function arraySearch(value, array, callback) {

callback = callback || false;

for (var i = 0; i < array.length; i++) {

if (array[i] == value) {

if (callback) {

return callback(value);

} else {

return value;

}

}

}

}

var result = arraySearch(4,[1,2,3,4],function(val){return val+val;});

At first glance it seems perfectly fine.

But let’s take a step back and use a preventative approach and focus instead on what the function shouldn’t do.

There are four points that we want to address in this exercise

It shouldn’t break easily. If at all possible we want to prevent it from stopping on error. Instead it should return.

2. It should never return undefined. We want it to return false instead.

3. It must never make implicit or “loose” match.

4. When we must throw an error it should not be a generic error. We want something readable for both ourselves and the poor programmer who needs to work on this code after us.

Getting Started

Point 1 seems like it’s asking a lot but in essence we just want it to fail gracefully and return a predictable value like false instead of stopping the bus.

First off, it absolutely must have an inputted value and array to run. So lets modify the function with this in mind.

function arraySearch(value, array, callback) {

if (value === undefined || array === undefined) {

return false;

}

callback = callback || false;

for (var i = 0; i < array.length; i++) {

if (array[i] == value) {

if (callback) {

return callback(value);

}

else {

return value;

}

}

}

}

Great, that’s sorted. By checking if the arguments are undefined we are ensuring that values have been passed to them.

Our callback already has a default value, so that is taken care of. But what if our array is not an array? Or in the same breath what if our callback is not a function?

Let’s take care of this next…

function arraySearch(value, array, callback) {

if (value === undefined || array === undefined || (array instanceof Array) === false) {

return false;

}

callback = callback || false;

if (callback !== false && typeof callback !== 'function') {

throw 'Callback to arraySearch is not a function';

return false;

}

for (var i = 0; i < array.length; i++) {

if (array[i] == value) {

if (callback) {

return callback(value);

}

else {

return value;

}

}

}

}

Awesome. Now by checking the typeof the callback we are sure that the callback is a valid function and by checking that the array is an instanceof the Array object we are also sure that the array is an Array.

So let’s move onto point 2 — “It should never return undefined”.

Well for starters our function does not have a default return value for when there is no match. Equally important, is the fact that we have no way of knowing what the callback function will return.

We can fix this by making the function return a variable so that we only need to check if it is undefined or null once.

function arraySearch(value, array, callback) {

if (value === undefined || array === undefined || (array instanceof Array) === false) {

return false;

}

callback = callback || false;

var result = null;

if (callback !== false && typeof callback !== 'function') {

throw 'Callback to arraySearch is not a function';

return false;

}

for (var i = 0; i < array.length; i++) {

if (array[i] == value) {

if (callback) {

result = callback(value);

}

else {

result = value;

}

}

}

return result || false;

}

Sorted. Setting the value of result to either the match or to the result of the callback function allows us to return either the result or false, should the result be undefined or null.

Point 3. An implicit or loose match can be described as being relatively equal i.e. false == 0 or ‘4’ == 4 etc.

We want to avoid this. What if we are searching for false in an Array containing Zero?

We can fix this by changing the below line:

if (array[i] == value) {

//must change to

if (array[i] === value) {

“===” means exactly equal to. Always do an explicit match when checking values. This habit will save you countless hours of time in the long run because you won’t be trying to debug statement that is evaluating as true.

Now for the last point.

When throwing an error we want it to be friendly. This functionality is already demonstrated when passing an invalid callback function, but what if a valid callback function throws an error?

Anonymous functions can be a pain to debug, so let’s try and make debugging a little less painful:

function arraySearch(value, array, callback) {

if (value === undefined || array === undefined || (array instanceof Array) === false) {

return false;

}

callback = callback || false;

var result = null;

if (callback !== false && typeof callback !== 'function') {

throw 'Callback to arraySearch is not a function';

return false;

}

for (var i = 0; i < array.length; i++) {

if (array[i] === value) {

if (callback) {

try{

result = callback(value);

}catch(e){

throw 'Callback function in arraySearch threw the error : '+e.message;

}

}

else {

result = value;

}

}

}

return result || false;

}

There we have it.

To solve the issue we use a simple try / catch statement and then re-throw the error with a custom message. Now if a callback function fails we will immediately know that it was the callback function that failed and not our arraySearch function.