17 Smells and Heuristics

Comments

  • C1: Inappropriate Information
    Information better held in a different kind of system such as your source code control system.
  • C2: Obsolete Comment
    A comment that has gotten old, irrelevant, and incorrect.
  • C3: Redundant Comment
    Describes something that adequately describes itself.
  • C4: Poorly Written Comment
    Use correct grammar and punctuation. Don’t ramble. Don’t state the obvious. Be brief.
  • C5: Commented-Out Code
    Delete it! Don’t worry, the source code control system still remembers it.

Environment

  • E1: Build Requires More Than One Step
    Building a project should be a single trivial operation.
  • E2: Tests Require More Than One Step
    Run all the unit tests with just one command.

Functions

  • F1: Too Many Arguments
    Functions should have a small number of arguments.
  • F2: Output Arguments
    Output arguments are counterintuitive.
  • F3: Flag Arguments
    Boolean arguments loudly declare that the function does more than one thing.
  • F4: Dead Function
    Methods that are never called should be discarded.

General

  • G1: Multiple Languages in One Source File
  • Minimize both the number and extent of extra languages in our source files.
  • G2: Obvious Behavior Is Unimplemented
  • Any function or class should implement the behaviors that another programmer could reasonably expect.
  • G3: Incorrect Behavior at the Boundaries
  • Look for every boundary condition and write a test for it.
  • G4: Overridden Safeties
  • It is risky to override safeties. (Turning off compiler warnings of failing tests).
  • G5: Duplication
  • Every duplication represents a missed opportunity for abstraction. Copies of the same code should be replaced with simple methods. Switch/case or if/else chains always testing for the same set of conditions should be replaced with polymorphism. Similar algorithms that don’t share similar lines of code should be addressed by using the TEMPLATE METHOD, or STRATEGY pattern.
  • G6: Code at Wrong Level of Abstraction
    Separate concepts at different levels and place them in different containers.
  • G7: Base Classes Depending on Their Derivatives
    In general, base classes should know nothing about their derivatives.
  • G8: Too Much Information
    Well-defined modules have very small interfaces that allow you to do a lot with a little.
  • G9: Dead Code
    Dead code is code that isn’t executed. After awhile it starts to smell.
  • G10: Vertical Separation
    Variables and function should be defined close to where they are used.
  • G11: Inconsistency
    If you do something a certain way, do all similar things in the same way.
  • G12: Clutter
    Keep your source files clean, well organized, and free of clutter.
  • G13: Artificial Coupling
    An artificial coupling is a coupling between two modules that serves no direct purpose.
  • G14: Feature Envy
    The methods of a class should be interested in the variables and functions of the class they belong to.
  • G15: Selector Arguments
    Selector arguments are just a lazy way to avoid splitting a large function into several smaller functions.
  • G16: Obscured Intent
    We want code to be as expressive as possible. (No Hungarian notation, magic numbers, etc.)
  • G17: Misplaced Responsibility
    Code should be placed where a reader would naturally expect it to be.
  • G18: Inappropriate Static
    Prefer nonstatic methods to static methods.
  • G19: Use Explanatory Variables
    Break calculations up into intermediate values that are held in variables with meaningful names.
  • G20: Function Names Should Say What They Do
    If you have to look at the implementation (or documentation) of the function to know what it does, then you should work to find a better name or rearrange the functionality.
  • G21: Understand the Algorithm
    Before you consider yourself to be done with a function, make sure you understand how it works.
  • G22: Make Logical Dependencies Physical
    The dependent module should not make assumptions about the module it depends upon.
  • G23: Prefer Polymorphism to If/Else or Switch/Case
    Follow the “ONE SWITCH” rule: There may be no more than one switch statement for a given type of selection.
  • G24: Follow Standard Conventions
    Every team should follow a coding standard based on common industry norms.
  • G25: Replace Magic Numbers with Named Constants
    But some constants are so easy to recognize that they don’t always need a named constant.
  • G26: Be Precise
    When you make a decision in your code, make sure you make it precisely.
  • G27: Structure over Convention
    Naming conventions are good, but they are inferior to structures that force compliance.
  • G28: Encapsulate Conditionals
    Extract functions that explain the intent of the conditional.
  • G29: Avoid Negative Conditionals
    Negatives are just a bit harder to understand than positives.
  • G30: Functions Should Do One Thing
    Functions that do more than one thing should be converted into many smaller functions.
  • G31: Hidden Temporal Couplings
    The order of some function calls is important, but the code does not enforce this temporal coupling.
  • G32: Don’t Be Arbitrary
    Have a reason for the way you structure your code, and make sure that reason is communicated by the structure of the code.
  • G33: Encapsulate Boundary Conditions
    Boundary conditions are hard to keep track of. Put the processing for them in one place.
  • G34: Functions Should Descend Only One Level of Abstraction
    Separating levels of abstraction is one of the most important functions of refactoring, and it’s one of the hardest to do well.
  • G35: Keep Configurable Data at High Levels
    If you have a constant such as a default or configuration value that is known and expected at a high level of abstraction, do not bury it in a low-level function.
  • G36: Avoid Transitive Navigation
    In general we don’t want a single module to know much about its collaborators.

Java

  • J1: Avoid Long Import Lists by Using Wildcards
    Long lists of imports are daunting to the reader.
  • J2: Don’t Inherit Constants
    Don’t use inheritance as a way to cheat the scoping rules of the language.
  • J3: Constants versus Enums
    Now that enums have been added to the language (Java 5), use them!

Names

  • N1: Choose Descriptive Names
    Take the time to choose names wisely and keep them relevant. Names are too important to treat carelessly.
  • N2: Choose Names at the Appropriate Level of Abstraction
    Don’t pick names that communicate implementation; choose names the reflect the level of abstraction of the class or function you are working in.
  • N3: Use Standard Nomenclature Where Possible
    Names are easier to understand if they are based on existing convention or usage.
  • N4: Unambiguous Names
    Choose names that make the workings of a function or variable unambiguous.
  • N5: Use Long Names for Long Scopes
    The length of a name should be related to the length of the scope.
  • N6: Avoid Encodings
    Names should not be encoded with type or scope information.
  • N7: Names Should Describe Side-Effects
    Don’t hide side effects with a name. Don’t use a simple verb to describe a function that does more than just that simple action.

Tests

  • T1: Insufficient Tests
    A test suite should test everything that could possibly break.
  • T2: Use a Coverage Tool!
    Coverage tools reports gaps in your testing strategy.
  • T3: Don’t Skip Trivial Tests
    They are easy to write and their documentary value is higher than the cost to produce them.
  • T4: An Ignored Test Is a Question about an Ambiguity
    We can express our question about the requirements as a test that is commented out, or as a test that annotated with @Ignore.
  • T5: Test Boundary Conditions
    We often get the middle of an algorithm right but misjudge the boundaries.
  • T6: Exhaustively Test Near Bugs
    Bugs tend to congregate. When you find a bug in a function, it is wise to do an exhaustive test of that function.
  • T7: Patterns of Failure Are Revealing
    Sometimes you can diagnose a problem by finding patterns in the way the test cases fail.
  • T8: Test Coverage Patterns Can Be Revealing
    Looking at the code that is or is not executed by the passing tests gives clues to why the failing tests fail.
  • T9: Tests Should Be Fast
    A slow test is a test that won’t get run.
Previous: 13 ConcurrencyUp: ContentsNext: –