Bad Comments :
The proper use of comments is to compensate for our failure to express ourself in code. Note that we used
the word failure. I meant it. Comments are always failures. We must have them because we cannot always
figure out how to express ourselves without them, but their use is not a cause for celebration.
There are certainly times when code makes a poor vehicle for explanation. Unfortunately, many programmers
have taken this to mean that code is seldom, if ever, a good means for explanation. This is patently false.
Which would you rather see? This:
// Check to see if the employee is eligible for full benefits
if ((employee.flags & HOURLY_FLAG) &&
(employee.age > 65))
Or this?
if (employee.isEligibleForFullBenefits())
Good Comments :
Some comments are necessary or beneficial. We’ll look at a few that I consider worthy of the bits they
consume. Keep in mind, however, that the only truly good comment is the comment you found a way not to
write. Sometimes a comment goes beyond just useful information about the implementation and provides the
intent behind a decision.
//This is our best attempt to get a race condition
//by creating large number of threads.
for (int i = 0; i < 25000; i++) {
WidgetBuilderThread widgetBuilderThread =
new WidgetBuilderThread(widgetBuilder, text, parent, failFlag);
Thread thread = new Thread(widgetBuilderThread);
thread.start();
}
assertEquals(false, failFlag.get());
}
Consider, for example, the procedural shape example in Listing 3-1. The Geometry class operates on the three shape classes. The shape classes are simple data structures without any behavior. All the behavior is in the Geometry class.
public class Square {
public Point topLeft;
public double side;
}
public class Rectangle {
public Point topLeft;
public double height;
public double width;
}
public class Circle {
public Point center;
public double radius;
}
public class Geometry {
public final double PI = 3.141592653589793;
public double area(Object shape) throws NoSuchShapeException {
if (shape instanceof Square) {
Square s = (Square) shape;
return s.side * s.side;
} else if (shape instanceof Rectangle) {
Rectangle r = (Rectangle) shape;
return r.height * r.width;
} else if (shape instanceof Circle) {
Circle c = (Circle) shape;
return PI * c.radius * c.radius;
}
throw new NoSuchShapeException();
}
}
Listing 3-1
Now consider the object-oriented solution in Listing 3-2. Here the area() method is polymorphic. No Geometry class is necessary. So if I add a new shape, none of the existing functions are affected, but if I add a new function all of the shapes must be changed!
public class Square implements Shape {
private Point topLeft;
private double side;
public double area() {
return side * side;
}
}
public class Rectangle implements Shape {
private Point topLeft;
private double height;
private double width;
public double area() {
return height * width;
}
}
public class Circle implements Shape {
private Point center;
private double radius;
public final double PI = 3.141592653589793;
public double area() {
return PI * radius * radius;
}
}
Listing 3.2
Procedural code (code using data structures) makes it easy to add new functions without changing the existing data structures. OO code, on the other hand, makes it easy to add new classes without changing existing functions.
Train Wrecks :
final String outputDir = ctxt.getOptions().getScratchDir().getAbsolutePath();
This kind of code is often called a train wreck because it look like a bunch of coupled train cars. Chains
of calls like this are generally considered to be sloppy style and should be avoided. It is usually best to
split them up as follows:
Options opts = ctxt.getOptions();
File scratchDir = opts.getScratchDir();
final String outputDir = scratchDir.getAbsolutePath();
Don't Return Null
public void registerItem(Item item) {
if (item != null) {
ItemRegistry registry = peristentStore.getItemRegistry();
if (registry != null) {
Item existing = registry.getItem(item.getID());
if (existing.getBillingPeriod().hasRetailOwner()) {
existing.register(item);
}
}
}
}
If you work in a code base with code like this, it might not look all that bad to you, but it is bad! When
we return null, we are essentially creating work for ourselves and foisting problems upon our callers. All
it takes is one missing null check to send an application spinning out of control.
In many cases,
special case objects are an easy remedy. Imagine that you have code like this:
List < Employee > employees = getEmployees();
if (employees != null) {
for (Employee e: employees) {
totalPay += e.getPay();
}
}
Right now, getEmployees can return null, but does it have to? If we change getEmployee so that it returns an empty list, we can clean up the code:
List < Employee > employees = getEmployees();
for (Employee e: employees) {
totalPay += e.getPay();
}
Fortunately, Java has Collections.emptyList(), and it returns a predefined immutable list that we can use for this purpose:
public List < Employee > getEmployees() {
if (..there are no employees..)
return Collections.emptyList();
}
Conclusion :
Clean code is readable, but it must also be robust.
These are not conflicting goals. We can write robust clean code if we see error handling as a separate
concern, something that is viewable independently of our main logic. To the degree that we are able to do
that, we can reason about it independently, and we can make great strides in the maintainability of our
code.
Source : Clean Code : A Handbook of Agile Software by Martin Fowler