Code Smells
Contents
Overview
A code smell is a surface indication that usually corresponds to a deeper problem in the system. — Martin Fowler
For a general list of common smells, see Jeff Atwood's article on code smells.
Common Odors
Dead Code
Just delete it. Version control will allow you to bring it back from the grave. If you wish to save it for future reference, bookmark a commit where it can be found.
Fluff Terms
Fluff terms are those little redundant words that end up variable names or method names or even URLs.
Examples:
# Bad Method Name accounts_information = user.accounts_information_from_service # Good Method Name service_accounts = user.accounts_from_service # Bad Variable Name affiliated_exchange_disclosure_step_file_handling # Good Variable Name affiliated_letter_upload_step # Bad Endpoint Path /api/v1/user/basic_info/:id # Good Endpoint Path /api/v1/user/:id
See also the Type Embedded in Name smell cited by Jeff Atwood:
Avoid placing types in method names; it's not only redundant, but it forces you to change the name if the type changes.
For some good advice on naming methods, see Semantic Method Naming.
Lurking Variables
These are long or complex conditions or expressions that could be clarified with an explaining temporary variable.
See also this StackExchange answer.
Magic Values
Magic values are unique or distinctive values buried in code. As Wikipedia states, "The use of unnamed magic [constants] in code obscures the developers' intent in choosing that value, increases opportunities for subtle errors (e.g. is every digit correct in 3.14159265358979323846 and is this equal to 3.14159?) and makes it more difficult for the program to be adapted and extended in the future."
Prefer constants or configuration settings.
Silent Errors
Silenced or swallowed errors can be an intense source of pain when trying to debug an issue. This article covers the topic well and is strongly recommended:
Also understand the advantages of "failing fast":
In short, when in active development, fail fast and loud.
String Concatenation
Prefer interpolation.
For guidance in Ruby, see the Rubocop Ruby Style Guide.
For PHP, prefer formatting with sprintf.
// Bad $auth_url = $this->auth::INFS_OAUTH_URL . '?client_id=' . $this->auth->get_client_id() . '&response_type=code&scope=full'; // Good $url_f = '%s?client_id=%s&response_type=code&scope=full'; $auth_url = sprintf($url_f, $this->auth::INFS_OAUTH_URL, $this->auth->get_client_id());
Unguarded Nested Conditional Clauses
See https://refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html.
Tests
Ambiguous Tests
Use expects pattern in test names and organize test methods around clear coherent, if not singular, expectations.
Disorganized Tests
Use the AAA or AAAA pattern:
# Arrange # Assume # Act # Assert
Surprises
Understand and apply the Principle of Least Surprise (POLA). If a reasonable developer reading your code will be surprised or have a questions, you probably want to add a code comment.
Related is the concept of affordances, popularized by Donald Norman in The Design of Everyday Things. It's also worth understanding as a developer.
