It smells because there are likely many instances where it could be edited or improved.
Most of these smells are just hints of something that might be wrong. They are not required fixed per se… ( You should look into it though.)
Let's continue...
result = ???
TL;DR: Use good names always. Result is always a very bad name.
var result;
result = lastBlockchainBlock();
//
// Many function calls
addBlockAfter(result);
var lastBlockchainBlock;
lastBlockchainBlock = findlastBlockchainBlock();
//...
// Many function calls
// we should refactor them to minimize space
// between variable definition and usage
addBlockAfter(lastBlockchainBlock);
We must forbid the word result to be a variable name.
Result is an example of generic and meaningless names.
Refactoring is cheap and safe.
Always leave the campground cleaner than you found it.
When you find a mess on the ground, clean it, doesn’t matter who did it. Your job is to always leave the ground cleaner for the next campers.
Code is like humor. When you have to explain it, it’s bad.
Cory House
Objects work fine and fulfill business objectives. But we need to test them. Let's break them.
TL;DR: Don't write methods with the only purpose of being used in your tests.
<?
class Hangman {
private $wordToGuess;
function __construct() {
$this->wordToGuess = getRandomWord();
//Test is not in control of this
}
public function getWordToGuess(): string {
return $this->wordToGuess;
//Sadly we need to reveal this
}
}
class HangmanTest extends TestCase {
function test01WordIsGuessed() {
$hangmanGame = new Hangman();
$this->assertEquals('tests', $hangmanGame->wordToGuess());
//how can we make sure the word is guessed?
}
}
<?
class Hangman {
private $wordToGuess;
function __construct(WordRandomizer $wordRandomizer) {
$this->wordToGuess = $wordRandomizer->newRandomWord();
}
}
class MockRandomizer implements WordRandomizer {
function newRandomWord(): string {
return 'tests';
}
}
class HangmanTest extends TestCase {
function test01WordIsGuessed() {
$hangmanGame = new Hangman(new MockRandomizer());
//We are in full control!
$this->assertFalse($hangmanGame->wordWasGuessed());
$hangmanGame->play('t');
$this->assertFalse($hangmanGame->wordWasGuessed());
$hangmanGame->play('e');
$this->assertFalse($hangmanGame->wordWasGuessed());
$hangmanGame->play('s');
$this->assertTrue($hangmanGame->wordWasGuessed());
//We just test behavior
}
}
This is a design smell.
We can detect we need a method just for test.
White-box tests are fragile. They test implementation instead of behavior.
This smell was inspired by @Rodrigo
Nothing makes a system more flexible than a suite of tests.
Robert Martin
Variable reuse is something we see in big chunks of code.
TL;DR: Don't reuse variable names. You break readability and refactor chances and gain nothing but laziness.
class Item:
def __init__(self, name):
self.name = name
def taxesCharged(self):
return 1;
class Money:
pass
lastPurchase = Item('Soda');
# Do something with the purchase
taxAmount = lastPurchase.taxesCharged();
# Lots of stuff related to the purchase
# I drink the soda
# I cannot extract method from below without passing
# useless lastPurchase as parameter
# a few hours later..
lastPurchase = Item('Whisky');
# I bough another drink
taxAmount += lastPurchase.taxesCharged();
class Item:
def __init__(self, name):
self.name = name
def taxesCharged(self):
return 1;
class Money:
pass
def buySupper():
supperPurchase = Item('Soda');
# Do something with the purchase
# Lots of stuff related to the purchase
# I drink the soda
return supperPurchase;
def buyDrinks():
# I could extract method!
# a few hours later..
drinksPurchase = Item('Whisky');
# I bough another drink
return drinksPurchase;
taxAmount = buySupper().taxesCharged() + buyDrinks().taxesCharged();
Many linters can warn us from reusing variables
Reusing variables is a non-contextual copy and paste hint.
Code Smell 03 - Functions Are Too Long
Photo by Robby McCullough on Unsplash
Either way you look at it (DRY or laziness), the idea is the same: make your program flexible. When change comes (and it always does), you'll have a much easier time changing with it.
Chris Pine
Some functions do not behave as expected. Sadly, most programmers accept them.
TL;DR: Don't trust max() and min() functions. Just ignore them.
console.log(Math.max() > Math.min());
//returns false
console.log(Math.max());
//returns -Infinite
console.log(Math.max() > Math.min());
console.log(Math.max());
//returns Exception. Not enough arguments passed.
//Max requires at least one argument
These functions belong to the standard Math library. Therefore, they are not easy to avoid.
We can block them on our linters.
We need to be very careful using functions that violate real-world concepts using language tricks.
Code Smell 69 - Big Bang (JavaScript Ridiculous Castings)
Photo by Cris Baron on Unsplash
Inspired by @@Oliver Jumpertz
Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the Universe trying to produce bigger and better idiots. So far, the Universe is winning.
Rick Cook
Do not perform more than requested.
TL;DR: Unless you need atomicity, do not perform more than one task.
def fetch_and_display_personnel():
data = # ...
for person in data:
print(person)
def fetch_personnel():
return # ...
def display_personnel(data):
for person in data:
print(person)
Functions including "and" are candidates. However, we need to check them carefully since there might be false positives.
We should avoid doing more than needed, and our functions should be both minimal and atomic.
This smell was inspired by
If it takes more than a sentence to explain what you are doing, it’s almost always a sign that what you are doing is too complicated.
Sam Altman
And that’s all for now…
The next article will explain 5 more code smells!