<<Clean Code>> Quotes: 17. Smells and Heuristics
As I made each change, I asked myself why I made that change and then wrote the reason down here. The result is a rather long list of things that smell bad to me when I read code.
Comments
C1: Inappropriate Information
It is inappropriate for a comment to hold information better held in a different kind of system such as your source code control system, your issue tracking system, or any other record-keeping system.
In general, meta-data such as authors, last-modified-date, SPR number, and so on should not appear in comments. Comments should be reserved for technical notes about the code and design.
C2: Obsolete comment
A omment that has gotten old, irrelevant, and incorrect is obsolete. Comments get old uickly. It is best not to write a comment that will become obsolete.
C3: Redundant Comment
A comment is redundant if it describes something that adequately describes itself.
Another example is a Javadoc that says nothing more than (or even less than) the function signature
Comments should say things that the code cannot say for itself.
C4: Poorly Written Comment
A comment worth writing is worth writing well.
C5: Commented-Out Code
When you see 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.
You should be able to check out the system with one simple command and the nissue 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.
Being able to run all the tests is so fundamental and so important that it should be quick, easy, and obvious to do.
Functions
F1: Too Many Arguments
Functions should have a small number of arguments.
F2: Output Arguments
Output arguments are counterintuitive.
If your function must change the state of something, have it change the state of the object it is called on.
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
The ideal is for a source file to contain one, and only one, language. realistically, we will probably have to use more than one. But we should take pains to minimize both the number and extent of extra languages in our source files.
G2: Obvious behavior Is Unimplemented
Following “The Principle of Least Surprise,” any function or class should implement the behaviors that another programmer could reasonably expect.
When an obvious behavior is not implemented, readers and users of the code can no longer depend on their intuition about function names. They lose their trust in the original author and must fall back on reading the details of the code.
G3: Incorrect Behavior at the Boundaries
The problem is that we seldom realize just how complicated correct behavior is.
There is no replacement for due diligence. Every boundary condition, every corner case, every quirk and exception represents something that can confound an elegant and intuitive algorithm. 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.
Turning off certain compiler warnings (or all warnings!) may help you get the build to succeed, but at the risk of endless debugging sessions. Turning off failing tests and telling yourself you’ll get them to pass later is as bad as pretending your credit cards are fee money.
G5: Duplication
Every time you see duplication in the code, it represents a missed opportunity for abstraction.
By folding the duplication into such an abstraction, you increase the vocabulary of the language of your design. Other programmers can use the abstract facilities you create. Coding becomes faster and less error prone because you have raised the abstraction level.
A more subtle form is the switch/case or if/else chain that appears again and again in various modules, always testing for the same set of conditions. These should be replaced with polymorphism.
Still more subtle are the modules that have similar algorithms, but that doesn’t share similar lines of code.
G6: Code at Wrong Level of Abstraction
It is important to create abstractions that separate higher level general concepts from lower level detailed concepts.
when we do this, we need to make sure that the separation is complete. We want all the lower level concepts to be in the derivatives and all the higher level concepts to be in the base class.
Good software design requires that we separate concepts at different levels and place them in different containers.
The point is that you cannot lie or fake your way out of a misplaced abstraction. Isolating abstractions is one of the hardest things that software developers do, and there is no quick fix when you get it wrong.
G7: Base Classes Depending on Their Derivatives
The most common reason for partitioning concepts into base and derivative classes is so that the higher level base class concepts can be independent of the lower level derivative class concepts.
In general, base classes should know nothing about their derivatives.
Somethings the number of derivatives is strictly fixed, and the base class has code that selects between the derivatives.
However, in that case the derivatives and base class are strongly coupled and always deploy together in the same jar file. In the general case we want to be able to deploy derivatives and bases in different jar files.
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.
This means that the impact of a change is greatly lessened, and maintaining systems in the field is made much simpler.
G8: Too Much Information
Well-defined modules have very small interfaces that allow you to do a lot with a little. Poorly defined modules have wide and deep interfaces that force you to use many different gestures to get simple things done. A well-defined interface does not offer very many functions to depend upon, so coupling is low.
Good software developers learn to limit what they expose at the interfaces of their classes and modules. The fewer methods a class has, the better. The fewer variables a function know about, the better. The fewer instance variables a class has, the better.
Hide your data. Hide your utility functions, Hide your constants and your temporaries. don’t create classes with lots of methods or lots of instance variables. Don’t create lots of protected variable 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 code that isn’t executed. You find it in the body of an if statement that checks for a condition that can’t happen. You find it in the catch block of a try that never throws. You find it in little utility methods that are never called or switch/case conditions that never occur.
Give it a decent burial. Delete it from the system.
G10: Vertical Separation
Variables and function should be defined close to where they are used.
We don’t want local variables declared hundreds of lines distant from their usages.
Private functions should be defined just below their first usage. we’d still like to limit the ertical distance between the invocations and definitions.
G11: Inconsistency
If you do something a certain way, do all similar things in the same way.
If within a particular function you use a variable named response to hold an HttpservletResponse, then use the same variable name consistently in the other functions that use HttpservletResponse objects. If you name a method processVerificationRequest, then use a similar name, such as processDeletionrequest, for the methods that process other kinds of requests.
G12: Clutter
Of what use is a default constructor with no implementation?
Variables that aren’t used, functions that are never called, comments that add no information, and so forth.
G 13: Artificial Coupling
Things that don’t depend upon each other should not be artificially coupled.
the same goes for general purpose static functions being declared in specific classes.
In general 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, and not the variables and functions of other classes. When a method uses accessors and mutators of some other object to manipulate the data within that object, then it envies the scope of the class of that other object.
All else being equal, we want to eliminate Feature Envy because it exposes the internals of one class to antoher. Sometimes, however, Feature Envy is a neccesary evil.
G15: Selector Arguments
There is hardly anything more abominable than a dangling false argument at the end of a function call.
Selector arguments are just a lazy way to avoid splitting a large function into several smaller functions.
Slectors need not be boolean. They can be enums, integers, or any other type of argument that is used to select the behavior of the function. In general it is better to have many functions than to pass some code into a function to select the behavior.
G16: Obscured Intent
We want code to be as expressive as possible. Run-on expressions, Hungarian notation, and magic numbers all obscure the author’s intent.
G17: Misplaced Responsibility
One of the most important decisions a software developer can make is where to put code.
Code should be placed where a reader would naturally expect it to be.
Sometimes we get “clever” about where to put ertain functionality. We’ll put it in a function that’s convenient for us, but not necessarily intuitive to the reader.
One way to make this decision is to look at the names of the functions.
G18: Inappropriate Static
In general you should prefer nonstatic methods to static methods. When in doubt, make the function nonstatic.
G19: Use Explanatory Variables
One of the more powerful ways to make a program readable is to break the calculations up into intermediate values that are held in variables with meaningful names.
It is hard to overdo this. More explanatory variables are generally better than fewer.
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 so that it can be placed in functions with better names.
G21: Understand the Algorithm
Lots of very funny code is written because people don’t take the time to understand the algorithm. They get something to work by plugging in enough if statements and flags, without really stopping to consider what is really going on.
Programming is often an exploration. You think you know the right algorithm for something, but then you wind up filling with it, prodding and poking at it, until you get it to “work.” How do you know it “works”? because it passes the test cases you can think of.
Before you consider youself to be done with a function, make sure you understand how it works.
G22: Make Logical Dependencies Physical
If one module depends upon another, that dependency should be physical, not just logical. the dependent module should not make assumptions(in other words, logical dependencies) about the module it depends upon. Rather it should explicitly ask that module for all the information it depends upon.
We can physicalize this dependency by creating a new method in HourlyreportFormatter named getMaPageSize(). HourlyReporter will then call that function rather than using the PAGE_SIZE constant.
G23: Prefer Polymorphism to If/Else or Switch/Case
First, most people use switch statements because it’s the obvious brute force solution, not because it’s the right solution for the situation. So this heuristic is here to remind us to consider polymorphism before using a switch.
Second, the cases where functions are more volatile than types are relatively rare. So every switch statement should be suspect.
I use the following “ONE SWITCH” rule: There may be no more tha none switch statement for a given type of selection. The cases in that switch statement must create polymorphic objects that take the place of other such switch statements in the rest of the system.
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. The team should not need a document to describe these conventions because their code provides the examples.
Everyone on the team should follow these conventions.
G25: replace magic Numbers with Named Constants
In general it is a bad idea to have raw numbers in your code. you should hide them behind well-named constants.
The term “Magic Number ”does not apply only to nubmers. It applies to any token that has a value that is not self-describing.
G26: Be Precise
When you make a decision in your code, make sure you make it precisely. Know why you have make it and how you will deal with any exception. Don’t be lazy about the precision of your decisions. If you decide to cal la function that might return null, make sure you check for null.
Ambiguities and imprecision in code are either a result of desagreements or laziness. In either case they should be eliminated.
G27: Structure over Convention
Enforce design decisions with structure over convention.
G28: Encapsulate Conditionals
G29: Avoid Negative Conditionals
When possible, conditionals should be expressed as positives.
G30: Functions Should Do One Things
It is often tempting to create functions that have multiple sections that perform a series of operations. Functions of this kind do more than one thing, and should be converted into many smaller functions, each of which does one thing.
G31: Hidden Temporal Couplings
Temporal couplings are often necessary, but you should not hide the coupling.
Each function produces a result that the next function needs, so there is no reasonable way to call them out of order.
You might complain that this increases the complexity of the functions, and you’d be right. But that extra syntactic complexity exposes the true temporal complexity of the situation.
G32: Don’t Be Arbitrary
Public classes that are not utilities of some other class should not be scoped inside another class. The convention is to make them public at the top level of their package.
G33: Encapsulate Boundary Conditions
Boundary conditions are hard to keep track of. Put the processing for them in one place. on’t let them leak all over the code.
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.
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 Hight 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. expose it as an argument to that low-level function called from the high-level function.
G36: Avoid Transitive Navigation
In general we don’t want a single module to know much about its collaborators.
This is sometimes called the Law of Demeter.
If many modules used some form of the statement a.getB().getC(), then it would be difficult to change the design and architecture it interpose a Q between B and C.
Rather we want out immediate collaborators to offer all the services we need.
Java
J1: Avoid Long Import Lists by Using Wildcards
J2: Don’t Inherit Constants
J3: Constants versus Enums
Names
N1: Choose Descriptive Names
Make sure the name is descriptive. Remember that meanings tend to drift as software evolves, so frequently reevaluate the appropriateness of the names you choose.
The power of carefully chosen names is that they overload the structure of the code with description. That overloading sets the reader’ expectations about what the other functions in the module do.
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.
Teams will often invent their own standard system of names for a particular project. Eric Evans refers to this as a ubiquitous language for the project.
N4: Unambiguous Names
N5: Use Long Names for Long Scopes
The length of a name should be related to the length of the scope.
On the other hand, variables and functions with short names lose their meaning over long distances.
N6: Avoid Encodings
Prefixes such as m_ or f are useless in today’s environments.
Again, today’s environments provide all that information without having to mangle the names.
N7: Names Should Describe Side-Effects
Don’t use a simple verb to describe a function that does more than just that simple action.
