Functions, lambdas, closures. So high order, nondeclarative, and hot.
TL;DR: Don't abuse closures and functions. Encapsulate them into objects.
Problems
-
Maintainability
- Testability
- Code Reuse
- Implementation Hiding
- Debugging
Solutions
Wrap functions/closures
Reify algorithms in method object / Strategy
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
Code Smell 06 - Too Clever Programmer
Maxi Contieri γ» Oct 25 '20 γ» 2 min read
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
Software Engineering Great Quotes
Maxi Contieri γ» Dec 28 '20 γ» 13 min read
This article is part of the CodeSmell Series.
How to Find the Stinky parts of your Code
Maxi Contieri γ» May 21 '21 γ» 8 min read
Last update: 2021/07/03
Top comments (8)
Given that "Humans read code", as you said, I find the first example wayyy easier to read than the second. The second example has over 2x as many lines and over 3x as many characters... I don't know if introducing classes was the right solution here, as it adds indirection and complexity. To me, it thus seems that Right is Wrong and Wrong is Right.
To modify the first example so you solve your 5 stated problems, you could simply:
Rename
sortFunction()βbubbleSort().Inside it (the sort function), rename
fnβcompare, for clarity, so the if sentence reveals the intent:if (compare(arr[j], arr[j+1]))Name the anonymous function before passing it into the (newly renamed) sort function:
I do agree that anonymous functions can and often are abused, though. But mostly because they are often injected everywhere (typically obfuscating param lists), without the developer having taken the the time to name and declare it (which would simplify the params list, and also reveal the function's intent better).
+100.
Maybe the problem exists (I canΚΌt say on its real spread), but the example shows quite opposite case.