Refactoring OR

Example refactoring a block of code containing OR condition

Problem: find if a flag was passed in the command line arguments. The flag can have several aliases.

node index.js --verbose
node index.js -v

We can of course use a dedicated 3rd party library to robustly handle command line options. But sometimes all we want is quick and dependency-free solution. Here is our first solution:

1
2
3
4
var verbose = process.argv.some(function (arg) {
return arg === '--verbose' || arg === '-v';
});
console.log('verbose?', verbose);

output

$ node index.js -v
verbose? true

This implementation certainly works, but it mixes 3 different features:

  • Iterating through process arguments
  • Checking if an argument matches given keyword
  • Combining several checks into result using OR

Mixing goals like this in single piece of code leads to complex and brittle code. Let us refactor this code into easy to read and test parts.

Split iteration

First refactoring: make argument check its own function

1
2
3
4
function isVerboseArgument(arg) {
return arg === '--verbose' || arg === '-v';
}
var verbose = process.argv.some(isVerboseArgument);

We separated two concerns: iterating through command line arguments and checking an individual argument.

Separate check

Second refactoring actually makes the code longer, but we are increasing the code clarity. We are replacing inlined string equalities with function calls

1
2
3
4
5
6
7
function isMatch(word, arg) {
return word === arg;
}
function isVerboseArgument(arg) {
return isMatch('--verbose', arg) || isMatch('-v', arg);
}
var verbose = process.argv.some(isVerboseArgument);

Having function isMatch will make next refactoring much simpler. Notice, that I am putting actual string value first, leaving the free arg parameter second. I generally recommend this order because this will simplify future partial application in left to right order.

Form each check

We can factor out each side of the OR condition using .bind

1
2
3
4
5
6
7
8
9
function isMatch(word, arg) {
return word === arg;
}
var isVerbose = isMatch.bind(null, '--verbose');
var isV = isMatch.bind(null, '-v');
function isVerboseArgument(arg) {
return isVerbose(arg) || isV(arg);
}
var verbose = process.argv.some(isVerboseArgument);

Introduce OR function

Just like we replaced inlined === with calling isMatch function, let us replace OR with a function

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
function isMatch(word, arg) {
return word === arg;
}
var isVerbose = isMatch.bind(null, '--verbose');
var isV = isMatch.bind(null, '-v');
function or(predicates) {
return function (arg) {
return predicates.some(function (predicate) {
return predicate(arg);
});
};
}
function isVerboseArgument(arg) {
return or([isVerbose, isV])(arg);
}
var verbose = process.argv.some(isVerboseArgument);

Let us refine or function to work with plain arguments without array

1
2
3
4
5
6
7
8
9
10
11
function or() {
var predicates = Array.prototype.slice.call(arguments);
return function (arg) {
return predicates.some(function (predicate) {
return predicate(arg);
});
};
}
function isVerboseArgument(arg) {
return or(isVerbose, isV)(arg);
}

This is very simple or function that passes single argument to each predicate function. We probably can transform it to accept multiple arguments, if needed

1
2
3
4
5
6
7
8
9
function or() {
var predicates = Array.prototype.slice.call(arguments);
return function () {
var orArguments = arguments;
return predicates.some(function (predicate) {
return predicate.call(null, orArguments);
});
};
}

Remove code

Finally, let us remove the code that is no longer necessary. Notice that isVerboseArgument is only calling or without doing anything else. Thus we can remove it, leaving only this simple code

1
2
// isMatch, isVerbose, isV, 'or' function like above
var verbose = process.argv.some(or(isVerbose, isV));

Each individual function isMatch, isVerbose, etc. has a single purpose and is pure - its output depends only on the inputs. The pure functions are simple to test. Pure functions that only have a single purpose are easy to test and to combine into more advanced logic.

In this particular case, we can move isMatch and or into a library, add unit tests (very simple for pure functions) and then reuse in our application. The final code or(isVerbose, isV) is really hard to mess accidentally, and should be simple to fix.

Alternative final solution

Let us say that you measured the previous solution, and looked at actual users entering command line arguments. You have noticed that most users prefer using -v to typing --verbose. In this case you can iterate through all arguments looking for -v first, then, only if not found, we will look for --verbose.

1
2
3
4
var anyVerbose = process.argv.some.bind(process.argv, isVerbose);
var anyV = process.argv.some.bind(process.argv, isV);
var findVerbose = or(anyVerbose, anyV); // 1
var isVerbose = findVerbose();

Again, we are creating 3 new functions, binding data to individual callbacks. Because we have already bound data, we can call the last function findVerbose without any arguments.

Alternative solution using Ramda

I do not like the previous solution - using only the array iterator methods included in EcmaScript5 does not really play nicely with functional style. You can use 3rd party functional library to make work with arrays simpler. For example, here is the same logic implemented using Ramda library

1
2
3
4
5
var R = require('ramda');
var anyVerbose = R.some(isVerbose);
var anyV = R.some(isV);
var findVerbose = or(anyVerbose, anyV);
var isVerbose = findVerbose(process.argv);

This is my favorite solution, partly because we are building all logic into findVerbose function. Then we call the function with given data.

Conclusion

You can only effectively refactor JavaScript code if you are comfortable creating functions. In the original code before refactoring we had a single anonymous callback function (line // 1)

1
2
3
var verbose = process.argv.some(function (arg) { // 1
return arg === '--verbose' || arg === '-v';
});

In the final result, we created several tiny functions. Some of them were created using closures (anonymous function returned from or), other functions were created by partially applying arguments (using .bind)

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
// explicit function
function isMatch(word, arg) {
return word === arg;
}
// create new functions using .bind
var isVerbose = isMatch.bind(null, '--verbose');
var isV = isMatch.bind(null, '-v');
function or() {
var predicates = Array.prototype.slice.call(arguments);
// return new function from this closure
return function (arg) {
return predicates.some(function (predicate) {
return predicate(arg);
});
};
}
// create callback function by calling `or(...)`
var verbose = process.argv.some(or(isVerbose, isV));

Returning new functions (as in or) and partial application (using .bind) are the basic blocks of functional programming.

Another important point is to clearly see the difference between creating a function and calling it. For example, or(isVerbose, isV) calls or which returns a function. This function (let us call it orPredicates is then passed to .some function that will call it once for each item (or until orPredicates returns true).

related: refactor AND