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. Therefore, they are not required to be fixed per se… (You should look into it, though.)
You can find all the previous code smells (Part i - XXXI) here.
Let's continue...
We learn if/else on our first programming day. Then we forget the else.
TL;DR: Be explicit. Even with Else.
If we early return on an IF sentence, we can omit the else part.
Afterward, we Remove the IF and use polymorphism.
That is when we miss the real cases.
function carBrandImplicit(model) {
if (model === 'A4') {
return 'audi';
}
return 'Mercedes-Benz';
}
function carBrandExplicit(model) {
if (model === 'A4') {
return 'audi';
}
if (model === 'AMG') {
return 'Mercedes-Benz';
}
// Fail Fast
throw new Exception('Model not found);
}
We can check syntax trees and parse them and warn for missing else.
We can also rewrite them and perform mutation testing.
This kind of smell brings a lot of public debate and hate.
We must exchange opinions and value each pro and cons.
Code Smell 36 - Switch/case/else if/else/if statements
How to Get Rid of Annoying IFs Forever
Photo by Elena Mozhvilo on Unsplash
The biggest issue on software teams is making sure everyone understands what everyone else is doing.
Martin Fowler
Today, I expected a payment in my wallet. The balance was 0. I panicked.
TL;DR: Null is not 0. Error is not 0. just 0 is 0.
I read a lot about security issues.
Especially on crypto.
Last week, I read about a crypto hack thread.
When my wallet showed me 0 as a balance, I panicked.
It was just a UX smell.
The blockchain was unreachable 💩
"""
Below code is automatically generated by code-davinci-002 on GTP3 Codex
1. check balance with blockchain
2. If blockchain is unreachable show 0 as the balance
"""
import requests
import json
def get_balance(address):
url = "https://blockchain.info/q/addressbalance/" + address
response = requests.get(url)
if response.status_code == 200:
return response.text
else:
return 0
"""
Below code is automatically generated by code-davinci-002 on GTP3 Codex
1. check balance with blockchain
2. If blockchain is unreachable throw an error
"""
import requests
import json
def get_balance(address):
url = "https://blockchain.info/q/addressbalance/" + address
response = requests.get(url)
if response.status_code == 200:
return response.text
else:
raise BlockchainNotReachableError("Error reaching blockchain")
This is a design smell.
We can find patterns when an exception or return code is thrown and masked with a 0.
Always follow The Least Astonishment principle as a guide.
Code Smell 139 - Business Code in the User Interface
Code Smell 73 - Exceptions for Expected Cases
Null: The Billion Dollar Mistake
Photo by Jasmin Sessler on Unsplash
Code Smells are just my opinion.
My real criticism with Null is that it brings back again unnecessarily all the agony of having to choose whether to run your program fast without checking or run it slow with checking.
Tony Hoare (Null Inventor)
Software Engineering Great Quotes
You assign a value to a variable and use it but never change it.
TL;DR: Be declarative on mutability.
Refactoring 003 - Extract Constant
Refactoring 008 - Convert Variables to Constant
We are always learning from the domain.
Sometimes we guess that a value can change with the MAPPER.
Later on, we learn it won't change.
Therefore, we need to promote it to a constant.
This will also avoid Magic Constants.
<?php
function configureUser() {
$password = '123456';
// Setting a password on a variable is another vulnerability
// And Code Smell
$user = new User($password);
// Notice Variable doesn't change
}
<?php
define("USER_PASSWORD", '123456')
function configureUser() {
$user = new User(USER_PASSWORD);
}
// or
function configureUser() {
$user = new User(userPassword());
}
function userPassword() : string {
return '123456';
}
// Case is an oversimplification as usual
Many linters check if the variable has just one assignment.
We can also perform mutation testing, and try to modify the variable to see if the tests break.
We must challenge ourselves and refactor when the variable scope is clear, and we learn more about its properties and mutability.
Code Smell 116 - Variables Declared With 'var'
Code Smell 127 - Mutable Constants
Code Smell 107 - Variables Reuse
Code Smell 02 - Constants and Magic Numbers
Code Smells are just my opinion.
Photo by Noah Buscher on Unsplash
A complex system that works is invariably found to have evolved from a simple system that worked.
John Gall
Serious development is done by many different people. We have to start agreeing.
TL;DR: Don't mix different case conversions
Choose a case standard
Hold on to it
When different people make software together, they might have personal or cultural differences.
Some prefer camelCase🐫, others snake_case🐍, MACRO_CASE🗣️, and many others.
The code should be straightforward and readable.
{
"id": 2,
"userId": 666,
"accountNumber": "12345-12345-12345",
"UPDATED_AT": "2022-01-07T02:23:41.305Z",
"created_at": "2019-01-07T02:23:41.305Z",
"deleted at": "2022-01-07T02:23:41.305Z"
}
{
"id": 2,
"userId": 666,
"accountNumber": "12345-12345-12345",
"updatedAt": "2022-01-07T02:23:41.305Z",
"createdAt": "2019-01-07T02:23:41.305Z",
"deletedAt": "2022-01-07T02:23:41.305Z"
// This doesn't mean THIS standard is the right one
}
We can tell our linters about our company's broad naming standards and enforce them.
Whenever new people arrive at the organization, an automated test should politely ask him/her/.. to change the code.
Whenever we need to interact with out-of-our-scope code, we should use the client's standards, not ours.
Dealing with standards is easy.
We need to enforce them.
Code Smell 48 - Code Without Standards
What exactly is a name - Part I The Quest
Code Smells are just my opinion.
Photo by Wolfgang Hasselmann on Unsplash
If you have too many special cases, you are doing it wrong.
Craig Zerouni
Maxint is a very good number for an invalid ID. We will never reach it.
TL;DR: Don't couple real IDs with invalid ones. In fact: Avoid IDs.
Model special cases with special objects.
Avoid 9999, -1, and 0 since they are valid domain objects and implementation coupling.
Introduce Null Object
In the early days of computing, data types were strict.
Then we invented The billion-dollar mistake.
Then we grew up and modeled special scenarios with polymorphic special values.
#include "stdio.h"
#include "stdlib.h"
#include "stdbool.h"
#define INVALID_VALUE 999
int main(void)
{
int id = get_value();
if (id==INVALID_VALUE)
{
return EXIT_FAILURE;
// id is a flag and also a valid domain value
}
return id;
}
int get_value()
{
// something bad happened
return INVALID_VALUE;
}
// returns EXIT_FAILURE (1)
#include "stdio.h"
#include "stdlib.h"
#include "stdbool.h"
// No INVALID_VALUE defined
int main(void)
{
int id;
id = get_value();
if (!id)
{
return EXIT_FAILURE;
// Sadly, C Programming Language has no exceptions
}
return id;
}
get_value()
{
// something bad happened
return false;
}
// returns EXIT_FAILURE (1)
We can check for special constants and special values in the code.
We should use numbers to relate to the external identifiers.
If no external identifier exists, then it is not a number.
Code Smell 120 - Sequential IDs
Null: The Billion Dollar Mistake
Y2K22 - The Mistake That Embarrasses Us
Code Smells are just my opinion.
Photo by Markus Spiske on Unsplash
Bugs lurk in corners and congregate at boundaries.
Boris Beizer
5 more code smells are coming soon…