Simple developer guides
This section discusses principles and practices that help to keep the source code of a class clean and easy to read.
Let's take a look at a bad, daft piece of code and see how it can be refactored based on simple principles in order to improve its quality. The following example class can saves an object into some storage (database, file , etc).
class DataSaver {
public void save(Object target) {
try {
// open connection
/*
Open connection code
...
*/
// write data
/*
Write data code
...
*/
// close connection
/*
Close connection code
...
*/
} catch (Exception e) {
/*
Error handling code
...
*/
}
}
}
This class has one public method that saves an object by doing the following things: opening a connection, writing data, closing the connection.
One of the first things one should take a look at is the class name. The name of the class seems to describe the purpose of class correctly (it sounds a little generic but given the abstract nature of this example it is fine). The next is the interface: the name of the public method is appropriate and the class doesn't expose unnecessary methods.
The main problem with this class is the save method. It's responsible for all the work and the error handling too. As a result it is long, and needs inline comments (starting with //) to make it easier understood. An obvious first step is to separate the "functional" code from the error handling:
class DataSaver {
public void save(Object target) {
try {
performSave(target);
} catch (Exception e) {
/*
Error handling code
...
*/
}
}
public void performSave(Object target) {
// open connection
/*
Open connection code
...
*/
// write data
/*
Data writing code
...
*/
// close connection
/*
Close connection code
...
*/
}
}
We were able to reduce the length of the save method and the error handling code is now clearly separated. The performSave is still doing all the work, but it can be easily split into 3 main steps:
class DataSaver {
public void save(Object target) {
try {
performSave(target);
} catch (Exception e) {
/*
Error handling code
...
*/
}
}
private void performSave(Object target) {
openConnection();
writeTarget(target);
closeConnection();
}
private void openConnection() {
/*
Open connection code
...
*/
}
private void writeTarget(Object target) {
/*
Data writing code
...
*/
}
private void closeConnection() {
/*
Close connection code
...
*/
}
}
The resulting class is easier to read. Reading performSave method alone can provide a brief overview what the class does. If someone is not interested in more details, can just stop reading at that level. All the methods have a clear purpose and they're short, harder to miss an error.
A general rule - with many exceptions of course - a method shouldn't consist of more than a few lines of code.
It's easy to notice the similarity between the method names and the comments in the original version. This is an important implication of this approach: the class doesn't need comments anymore, the methods comment themselves with their names. Inline comments are often viewed as part of high quality code. Actually, more often the not, the exact opposite is true: comments are often just to save the day for badly written code. Because of that, they can be used as hints on what pieces of codes to extract to methods.
Note that all methods perform steps on the same abstraction level: openConnection, writeTarget, closeConnection are all relatively high-level steps. Jumping between different levels of abstraction is more difficult for the brain to follow. Because of that, sometimes it even worths to introduce small, even single-line methods.
There is an element that looks suspicious: passing the "target" argument so many times. Extracting methods usually requires more arguments to pass. In general the more argument a method takes, the less easy to read. Passing one or two arguments may be ok, but passing more makes the code considerably harder to read.
An option to address this is using the state of the object instead of passing arguments:
class DataSaver {
private Object target;
public void save(Object target) {
try {
this.target = target;
performSave();
} catch (Exception e) {
/*
Error handling code
...
*/
}
}
private void performSave() {
openConnection();
writeTarget();
closeConnection();
}
private void openConnection() {
/*
Open connection code
...
*/
}
private void writeTarget() {
/*
Write data code
...
*/
}
private void closeConnection() {
/*
Close connection code
...
*/
}
}
Note: using the object state to replace argument passing can make an originally thread-safe class non-thread-safe. This is not a big problem as long threads use different instances.
class DataSaver {
private DataAccess access;
public void save(Object target) {
access = new DataAccess();
access.open();
writeData(target);
access.close();
}
private void writeData(Object target) {
/*
...
access.write(...);
access.write(...);
...
*/
}
}
class DataAccess {
public void open() {
/*
Open connection code
...
*/
}
public void close() {
/*
Open connection code
...
*/
}
public void write(...) {
/*
Writing to the connected resource
...
*/
}
}
As a result the DataSaver class has been greatly simplified. The DataAccess class took over all the details of handling the connection of the external resource. It is also potentially a reusable class.
Read the next guide: value objects