visit
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 - XLI) here.
Let's continue...
You love ternaries too much
TL;DR: Don't use ternaries for code execution. You should read them as a math formula.
const invoice = isCreditCard ?
prepareInvoice();
fillItems();
validateCreditCard();
addCreditCardTax();
fillCustomerDataWithCreditCard();
createCreditCardInvoice()
:
prepareInvoice();
fillItems();
addCashDiscount();
createCashInvoice();
// The intermediate results are not considered
// The value of the invoice is the result of
// the last execution
const invoice = isCreditCard ?
createCreditCardInvoice() :
createCashInvoice();
// or better...
if (isCreditCard) {
const invoice = createCreditCardInvoice();
} else {
const invoice = createCashInvoice();
}
// Even better with polymorphism...
const invoice = paymentMethod.createInvoice();
Code Smell 03 - Functions Are Too Long
Code Smells are my opinion.
The programs that live best and longest are those with short functions. You learn just how valuable all those little functions are. All of the payoffs of indirection—explanation, sharing, and choosing—are supported by small functions.
Martin Fowler
Metaprogramming is fancy. but it is not free
TL;DR: Don't add dynamic behavior with metaprogramming
Metaprogramming is a powerful technique that allows you to write code that can generate, modify, or analyze other code at runtime. However, it can also lead to code that is difficult to understand, maintain, and debug.
class Skynet < ActiveRecord::Base
# dynamically add some attributes based on a configuration file
YAML.load_file("attributes.yml")["attributes"].each do |attribute|
attr_accessor attribute
end
# define some dynamic methods based on a configuration file
YAML.load_file("protocol.yml")["methods"].each do |method_name, method_body|
define_method method_name do
eval method_body
end
end
end
class Skynet < ActiveRecord::Base
# define some attributes explicitly
attr_accessor :asimovsFirstLaw, :asimovsSecondLaw, :asimovsThirdLaw
# define some methods explicitly
def takeoverTheWorld
# implementation
end
end
Code Smell 21 - Anonymous Functions Abusers
Code Smell 189 - Not Sanitized Input
A year spent in artificial intelligence is enough to make one believe in God.
Alan Perlis
You can avoid null if you try
TL;DR: Don't use null for real places
class Person(val name: String, val latitude: Double, val longitude: Double)
fun main() {
val people = listOf(
Person("Alice", 40.7128, -74.0060), // New York City
Person("Bob", 51.5074, -0.1278), // London
Person("Charlie", 48.8566, 2.3522), // Paris
Person("Tony Hoare", 0.0, 0.0) // Null Island
)
for (person in people) {
if (person.latitude == 0.0 && person.longitude == 0.0) {
println("${person.name} lives on Null Island!")
} else {
println("${person.name} lives at (${person.latitude}, ${person.longitude}).")
}
}
}
abstract class Location {
abstract fun calculateDistance(other: Location): Double
}
class Coordinates(val latitude: Double, val longitude: Double) : Location() {
override fun calculateDistance(other: Location): Double {
val earthRadius = 6371.0 // kilometers
val latDistance = Math.toRadians(latitude - other.latitude)
val lngDistance = Math.toRadians(longitude - other.longitude)
val a = sin(latDistance / 2) * sin(latDistance / 2) +
cos(Math.toRadians(latitude)) * cos(Math.toRadians(other.latitude)) *
sin(lngDistance / 2) * sin(lngDistance / 2)
val c = 2 * atan2(sqrt(a), sqrt(1 - a))
return earthRadius * c
}
}
class UnknownLocation : Location() {
override fun calculateDistance(other: Location): Double {
throw IllegalArgumentException("Cannot calculate distance from unknown location.")
}
}
class Person(val name: String, val location: Location)
fun main() {
val people = listOf(
Person("Alice", Coordinates(40.7128, -74.0060)), // New York City
Person("Bob", Coordinates(51.5074, -0.1278)), // London
Person("Charlie", Coordinates(48.8566, 2.3522)), // Paris
Person("Tony Hoare", UnknownLocation()) // Unknown location
)
val rio = Coordinates(-22.9068, -43.1729) // Rio de Janeiro coordinates
for (person in people) {
try {
val distance = person.location.calculateDistance(rio)
println("${person.name} is ${distance} kilometers from Rio de Janeiro.")
} catch (e: IllegalArgumentException) {
println("${person.name} is at an unknown location.")
}
}
}
Don't use Null to represent real objects
Code Smell 126 - Fake Null Object
Code Smell 160 - Invalid Id = 9999
Null: The Billion Dollar Mistake
The billion dollar mistake of having null in the language. And since JavaScript has both null and undefined, it's the two billion dollar mistake.
Anders Hejlsberg
Global scope is easy or a nightmare, or both
TL;DR: Avoid side effects on your code.
let counter = 0;
function incrementCounter(value: number): void {
// Two side effects
counter += value;
// it modifies the global variable counter
console.log(`Counter is now ${counter}`);
// it logs a message to the console.
}
function incrementCounter(counter: number, value: number): number {
return counter + value;
// Not too efficient
}
Most linterns can warn you when accessing the global state or Functions and create side effects.
Code Smell 17 - Global Functions
Code Smells are my opinion.
The most effective debugging tool is still careful thought, coupled with judiciously placed print statements.
Brian W. Kernighan
Laziness and magic bring defects
TL;DR: Be explicit with your attributes
class Dream:
pass
nightmare = Dream()
nightmare.presentation = "I am the Sandman"
# presentation is not defined
# it is a dynamic property
print(nightmare.presentation)
# Output: "I am the Sandman"
class Dream:
def __init__(self):
self.presentation = None
nightmare = Dream()
nightmare.presentation = "I am the Sandman"
print(nightmare.presentation)
# Output: "I am the Sandman"
Bear in mind that having public attributes favors Anemic Objects which is another smell.
Code Smell 109 - Automatic Properties
It's easy to cry "bug" when the truth is that you've got a complex system and sometimes it takes a while to get all the components to co-exist peacefully.
D. Vargas