Skip to content

Smells and Heuristics

Comments

C1: Inappropriate Information

Do not write Inappropriate comments. Comments should be reserved for technical notes about the code and design.

C2: Obsolete Comment

Do not write gotten old, irrelevant, and incorrect comments.

C3: Redundant Comment

Do not write unnecessary comments

C4: Poorly Written Comment

If you are going to write a comment, take the time to make sure it is the best comment you can write. And also check grammer and punctuation.

C5: Commented-Out Code

Do not write commented out code.

Environment

E1: Build Requires More Than One Step

Bulid a project in single trivial operation.You should be able to check out the system with one simple command and then issue one other simple command to build it.

E2: Tests Require More Than One Step

You should be able to run all the unit tests with just one command.

Functions

F1: Too Many Arguments

Functions should have a small number of arguments. No argument is best, followed by one, two, and three. More than three is very questionable and should be avoided with prejudice.

F2: Output Arguments

Do not take output as an argument.

F3: Flag Arguments

Do not take flag as argument

F4: Dead Function

Methods that are never called should be discarded.

General

G1: Multiple Languages in One Source File

The ideal is for a source file to contain one, and only one, language. Do not write multiple languages in one source file.

G2: Obvious Behavior Is Unimplemented

Do not write obvious Behavior is un implemented.

Eg: we have one enum that represents day . Day day = DayDate.StringToDay(String dayName);

We would expect the string "Monday" to be translated to Day.MONDAY . We would also expect the common abbreviations to be translated, and we would expect the function to ignore case.

G3: Incorrect Behavior at the Boundaries

Don’t rely on your intuition. Look for every boundary condition and write a test for it.

G4: Overridden Safeties

It is risky to override safeties. Exerting manual control over serialVersionUID may be necessary, but it is always risky.

Turning off certain compiler warnings (or all warnings!) may help you get the build to succeed, but at the risk of endless debugging sessions.

G5: Duplication

Do not write duplication of code.

Do not write switch/if statements.these should be replaced with polymorphism.

Still more subtle are the modules that have similar algorithms, but that don’t share similar lines of code. This is still duplication and should be addressed by using the TEMPLATE METHOD , or STRATEGY pattern.

G6: Code at Wrong Level of Abstraction

It is important to create abstractions that separate higher level general concepts from lower level detailed concepts. Sometimes we do this by creating abstract classes to hold the higher level concepts and derivatives to hold the lower level concepts.

G7: Base Classes Depending on Their Derivatives

Base class do not depending on their derivatives.

Deploying derivatives and bases in different jar files and making sure the base jar files know nothing about the contents of the derivative jar files allow us to deploy our systems in discrete and independent components.

When such components are modified, they can be redeployed without having to redeploy the base components.

G8: Too Much Information

Well-defined modules have very small interfaces that allow you to do a lot with a little.

Hide your data, utility functions, constants and temporaries.

Don’t create classes with lots of methods or lots of instance variables. Don’t create lots of protected variables and functions for your subclasses.

Concentrate on keeping interfaces very tight and very small. Help keep coupling low by limiting information.

G9: Dead Code

Dead code is a code that never excuted. Do not write deadcode.

G10: Vertical Separation

Variables and function should be defined close to where they are used.

Local variables should be declared just above their first usage and should have a small vertical scope.

Private functions should be defined just below their first usage.

G11: Inconsistency

Do not write inconsistency data.

Simple consistency like this, when reliably applied, can make code much easier to read and modify.

G12: Clutter

Variables that aren’t used, functions that are never called, comments that add no information, and so forth. All these are clutter .so do not write clutter data.

G13: Artificial Coupling

Things that Do not depend upon each other should not be artificially coupled.

G14: Feature Envy

And then there’s feature envy. If a class seems to do very little except it uses all the methods of one other class, it’s another sign that you need to rethink the roles of one or the other.

G15: Selector Arguments

Do not write selector arguments.

G16: Obscured Intent

Expresses your intent in code.

G17: Misplaced Responsibility

Code should be placed where a reader would naturally expect it to be.

G18: Inappropriate Static

G19: Use Explanatory Variables

use meaning full names for variables.

G20: Function Names Should Say What They Do

function names should tell what they function do.

G21: Understand the Algorithm

Take time to understand the algorithm.

G22: Make Logical Dependencies Physical

If one module depends upon another, that dependency should be physical, not just logical.

G23: Prefer Polymorphism to If/Else or Switch/Case

Do not write if/else or switch/ case instead of that prefer polymorphism.

G24: Follow Standard Conventions

Every team should follow a coding standard based on common industry norms.

This coding standard should specify things like where to declare instance variables; how to name classes, methods, and variables; where to put braces; and so on.

G25: Replace Magic Numbers with Named Constants

Do not write raw numbers in your instead of that write well-named constants.

G26: Be Precise

Don’t be lazy about the precision of your decisions. If you decide to call a function that might return null , make sure you check for null.

G27: Structure over Convention

Enforce design decisions with structure over convention. Naming conventions are good, but they are inferior to structures that force compliance.

For example, switch/cases with nicely named enumerations are inferior to base classes with abstract methods.

No one is forced to implement the switch / case statement the same way each time; but the base classes do enforce that concrete classes have all abstract methods implemented.

G28: Encapsulate Conditionals

Extract functions that explain the intent of the conditional.

For example:

1
if (shouldBeDeleted(timer))

is preferable to

1
if (timer.hasExpired() && !timer.isRecurrent())

G29: Avoid Negative Conditionals

Negatives are just a bit harder to understand than positives. So, when possible, conditionals should be expressed as positives. For example:

1
if (buffer.shouldCompact())

is preferable to

1
if (!buffer.shouldNotCompact())

G30: Functions Should Do One Thing:

write like this

G31: Hidden Temporal Couplings

Do not hide the 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.

If a structure appears arbitrary, others will feel empowered to change it. If a structure appears consistently throughout the system, others will use it and preserve the convention.

G33: Encapsulate Boundary Conditions

Boundary conditions are hard to keep track of. Put the processing for them in one place.

Don’t let them leak all over the code. We don’t want swarms of +1 s and -1 s scattered hither and yon.

G34: Functions Should Descend Only One Level of Abstraction

The statements within a function should all be written at the same level of abstraction, which should be one level below the operation described by the name of the function.

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 write it in a low-level function.

Expose it as an argument to that low-level function called from the high-level function.

G36: Avoid Transitive Navigation

Do not write like this a.getB().getC().doSomething(); instead of that write like this myCollaborator.doSomething().

Java

J1: Avoid Long Import Lists by Using Wildcards

Do not write long import lists instead of that write wildcard imports.

J2: Don’t Inherit Constants

Do not inherit constants from interface.instead of that take static import.

J3: Constants versus Enums

Don’t keep using the old trick of public static final int s. Instead of that use Enums

Names

N1: Choose Descriptive Names

N2: Choose Names at the Appropriate Level of Abstraction

N3: Use Standard Nomenclature Where Possible

Names are easier to understand if they are based on existing convention or usage.

For example, if you are using the D ECORATOR pattern, you should use the word Decorator in the names of the decorating classes.

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. You can use very short variable names for tiny scopes, but for big scopes you should use longer names.

N6: Avoid Encodings

Do not use encoding Standrads

N7: Names Should Describe Side-Effects

Names should describe everything that a function, variable, or class is or does. 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

Do not write insufficient test.

T2: Use a Coverage Tool!

Coverage tools reports gaps in your testing strategy. They make it easy to find modules, classes, and functions that are insufficiently tested.

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

Sometimes we are uncertain about a behavioral detail because the requirements are unclear.

We can express our question about the requirements as a test that is commented out, or as a test that annotated with @Ignore .

Which you choose depends upon whether the ambiguity is about something that would compile or not.

T5: Test Boundary Conditions

Take special care to 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. You’ll probably find that the bug was not alone.

T7: Patterns of Failure Are Revealing

Sometimes you can diagnose a problem by finding patterns in the way the test cases fail.

This is another argument for making the test cases as complete as possible. Complete test cases, ordered in a reasonable way, expose patterns.

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. When things get tight, it’s the slow tests that will be dropped from the suite. So do what you must to keep your tests fast.