Code Smells

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
service_disclosure_step_file_handling

# Good Variable Name
service_disclosure_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:

• https://realpython.com/the-most-diabolical-python-antipattern/

Also understand the advantages of "failing fast":

• https://en.wikipedia.org/wiki/Fail-fast#Hardware_and_software

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.

Deodorizers

• Rubocop
• PHP CodeSniffer

References

• https://martinfowler.com/bliki/CodeSmell.html
• https://blog.codinghorror.com/code-smells/
• https://en.wikipedia.org/wiki/Code_smell