Both Kinds of Music

social-media-masters-buzz-evangelism-and-word-of-mouth-sean-moffitt-34-728

It’s not a rare thing to find something like this in a codebase.

const addElements = (elements, count, ascending) => {
   const toAdd = _.slice(elements, count);
   if (ascending) {
     // add them in ascending order
     return _.reduce(toAdd, (a, b) => a + b, '');
   }
   // add them in reverse order
   return _.reduce(_.reverse(toAdd), (a, b) => a + b, '');
}

That boolean variable “ascending” is a clue. It’s a clue that someone was trying to pack two unrelated outcomes into the same function. The idea that a function’s behaviour changes based on the caller’s compile-time/development-time (if you’re in a non-compiled language) choice, is ludicrous, but very easily done.

If you find yourself making a function dependent on how the caller wants it to run, then you should really consider splitting into two functions.

const addElements = (elements, count) => {
   const toAdd = _.slice(elements, count);

   // add them in ascending order
   return _.reduce(toAdd, (a, b) => a + b, '');
}

const addElementsInReverse = (elements, count) => {
   const toAdd = _.slice(elements, count);

   // add them in reverse order
   return _.reduce(_.reverse(toAdd), (a, b) => a + b, '');
}

This has the unfortunate side effect of duplicating code, so there’s a workaround for that. I say workaround. In some cases, it’s a code improvement, in other cases, it’s a neat refinement of the code to represent the polymorphism of the implementation. It may obfuscate your code, or it may clarify it. The possible approach is to facade the multiple incoming functions and have each function at the facade call a lower-level function passing some sort of function, or polymorphic thing which implements the difference between one use case and the next. E.g.

const addElementsInternal = (elements, count, preProcess) => {
   const toAdd = _.slice(elements, count);

   // add them after preprocessing
   return _.reduce(preProcess(toAdd), (a, b) => a + b, '');
}

const addElement = (elements, count) => 
  addElementsInternal(elements, count, _.identity);

const addElementsInReverse = (elements, count) =>
  addElementsInternal(elements, count, _.reverse);

 

To clarify, the issue solved by the above is the multiple lower-level implementations hiding within the original function. The decision of how to sequence the list is being made at runtime every time. Passing in a pre-processor to do that for a core internal function, avoids it having to make that decision and get bogged down in multiple lower-level concerns. Dividing the two use cases into two differently named or overloaded functions (dependent on language) avoids a murky one-size-fits-all function.

Examples use lodash.

And for fans:

Advertisements

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