Skip to main content

Command Palette

Search for a command to run...

Code Smell 21 - Anonymous Functions Abusers

Published
2 min read
Code Smell 21 - Anonymous Functions Abusers
M

I’m a senior software engineer loving clean code, and declarative designs. S.O.L.I.D. and agile methodologies fan.

Functions, lambdas, closures. So high order, non-declarative, and hot.

TL;DR: Don't abuse closures and functions. Encapsulate them into objects.

Problems

  • Maintainability
  • Testability

  • Code Reuse

  • Implementation Hiding

  • Debugging

Solutions

  1. Wrap functions/closures

  2. Reify algorithms in a method object / Strategy

Refactorings

Sample Code

Wrong

sortFunction = function(arr, fn) {
  var len = arr.length;

  for (var i = 0; i < len ; i++) {
    for(var j = 0 ; j < len - i - 1; j++){  
      if (fn(arr[j], arr[ j+ 1])) {
        var temp = arr[j];
        arr[j] = arr[j+1];
        arr[j + 1] = temp;
      }
    }
  }
  return arr;
}

scores = [9, 5, 2, 7, 23, 1, 3];  
sorted = sortFunction(scores, (a,b) => {return a > b});

Right

class ElementComparator{
  greatherThan(firstElement, secondElement){
    return firstElement > secondElement;
    //This is just an example. With more complex objects this comparison might not be trivial
  }
}

class BubbleSortingStrategy {
  //We have a strategy, we cant unit test it, change for a polymorphic,
  //Swap and benchmark algorithms etc.
  constructor(collection, comparer){
    this._elements = collection;
    this._comparer = comparer;
  }
  sorted(){ 
    for (var outerIterator = 0; outerIterator < this.size() ; outerIterator++) {
      for(var innerIterator = 0 ; innerIterator < this.size() - outerIterator - 1; innerIterator++){  
        if (this._comparer.greatherThan(this._elements[innerIterator], this._elements[ innerIterator + 1])) {
          this.swap(innerIterator);  
        }
      } 
    } 
    return this._elements; 
  }
  size() {
    return this._elements.length;
  }

  swap(position){    
    var temporarySwap = this._elements[position];
    this._elements[position] = this._elements[position + 1];
    this._elements[position + 1] = temporarySwap;
  }
} 

scores = [9, 5, 2, 7, 23, 1, 3]; 
sorted = new BubbleSortingStrategy(scores,new ElementComparator()).sorted();

Detection

  • Closures and anonymous functions are very useful to model code blocks, promises etc. So It'd difficult to tear them apart.

Tags

  • Primitive

  • Abuser

Conclusion

Humans read code. Software works ok with anonymous functions, but maintainability is compromised when multiple closures are invoked.

Relations

Credits

Photo by Roman Mager on Unsplash


Object-oriented programming increases the value of these metrics by managing this complexity. The most effective tool available for dealing with complexity is abstraction. Many types of abstraction can be used, but encapsulation is the main form of abstraction by which complexity is managed in object-oriented programming.

Rebecca Wirfs-Brock


This article is part of the CodeSmell Series.

Last update: 2021/07/03

T

I am now an official fan of your Code Smell explanations! Great job.

1
D

These busters are the most hard to debug! Not like debugging was an option, if code smells very much to begin with 😏

1
M

Agreed!

Most debuggers are very dumb related to closures and source code.

I personally prefer unit testing instead of debugging and this is also hard with this smell.

1

Code Smells

Part 1 of 50

In this series, we will see several symptoms and situations that make us doubt the quality of our developments. We will present possible solutions. Most are just clues. They are no hard rules.