Thinking when reading source code

Recently, while looking at the source code for thread pools under the Concurrent package, I noticed a problem with the JDK source code when I looked at the ThreadPoolExecutor class. Here is a snippet of the addWorker method of the ThreadPoolExecutor class:

boolean workerStarted = false;
boolean workerAdded = false;
Worker w = null;
try {
    w = new Worker(firstTask);
    final Thread t = w.thread;
    if(t ! =null) {
        final ReentrantLock mainLock = this.mainLock;
        mainLock.lock();
        try {
            int rs = runStateOf(ctl.get());

            if (rs < SHUTDOWN ||
                (rs == SHUTDOWN && firstTask == null)) {
                if (t.isAlive()) // precheck that t is startable
                    throw new IllegalThreadStateException();
                workers.add(w);
                int s = workers.size();
                if (s > largestPoolSize)
                    largestPoolSize = s;
                workerAdded = true; }}finally {
            mainLock.unlock();
        }
        if (workerAdded) {
            t.start();
            workerStarted = true; }}}finally {
    if (! workerStarted)
        addWorkerFailed(w);
}
return workerStarted;
Copy the code

The functionality of this code is perfectly fine, but the code is much more readable if you use guard statements. So what is a guard statement?

What is a guard statement?

Conditional expressions usually take two forms. The first form is: all branches are normal behavior. In the second form, only one of the answers provided by the conditional expression is normal behavior, and the rest are unusual cases. These two types of conditional expressions have different purposes, which should be shown in the code.

If both branches are normal behavior, use something like if… else… Conditional expression of; If a condition is extremely rare, it should be checked separately and returned from the function as soon as it is true. Such separate checks are often referred to as’ guard clauses. ‘ It’s hard to understand just by looking at the concepts, so let’s take two examples.

Conditional check replacement

This is a method of calculating employee compensation with special rules for expatriate and retired employees. These situations are rare, but they do occur occasionally.

public double getSalary(a) {
    double result;
    if (this.isSeparated) {// Overseas staff
        result = this.separatedSalary();
    } else {
        if (this.isRetired) {// Retired employees
            result = this.retiredSalary();
        } else {// Regular staff
            result = this.normalSalary(); }}return result;
}
Copy the code

In this code, the normal condition checks are overshadowed by the normal condition checks, so these condition checks should be replaced with a sanitary statement to improve program clarity. For each check, put in a guard statement. The statement either returns from the function or throws an exception.


public double getSalary(a) {
    if (this.isSeparated) {/ / who statement
        return this.separatedSalary();
    }
    if (this.isRetired) {/ / who statement
        return this.retiredSalary();
    }
    return this.normalSalary();
}
Copy the code

Reversed conditional substitution

This is a method to find the volume of cuboid with known length, width and height, but there is a special requirement: when the height is greater than 0, print the Society of Ten thousand cats. (Surprise? Are you surprised? Does it stand out? Become abnormal? Yes, sometimes we get requests like this.) The code looks like this:

public double getVolume(double length, double width, double height) {
    double result = 0.0;
    if (height > 0.0) {
        System.out.println(The Society of Ten Thousand Cats);
        if (length > 0.0 && width > 0.0) { result = length * width * height; }}return result;
}
Copy the code

Again, replace the condition check with a guard statement, but we need to reverse the corresponding condition, that is, do the logical non-operation.

public double getVolume(double length, double width, double height) {
    if (height <= 0.0) {// select * from (); (height > 0)
        return 0.0;
    }
    System.out.println(The Society of Ten Thousand Cats);
    if (length <= 0.0 || width <= 0.0) {// select * from (); (length > 0 && width > 0)
        return 0.0;
    }
    return length * width * height;
}
Copy the code

Why use guard statements?

The essence of the guard statement is to give special attention to a particular branch. If you use if… else… Structure, you care about the if branch as much as the else branch. The message this code structure conveys to the reader is that all branches are equally important.

The wei statement, on the other hand, tells readers: “This situation is rare, and if it does occur, please do the necessary tidied up and exit.” If you are no longer interested in the rest of the method, you should certainly quit immediately. Directing the reader of your code to a useless else section only hinders their understanding. The code is easier to understand and maintain with the use of guard statements.

Optimizing JDK code

With the above explanation, the addWorker method snippet can be optimized to:

boolean workerStarted = false;
boolean workerAdded = false;
Worker w = null;
try {
    w = new Worker(firstTask);
    final Thread t = w.thread;
    if (t == null) {/ / who statement
        return workerStarted;
    }

    final ReentrantLock mainLock = this.mainLock;
    mainLock.lock();
    try {
        int rs = runStateOf(ctl.get());

        / / who statement
        if(rs > SHUTDOWN || (rs == SHUTDOWN && firstTask ! =null)) {
            return workerStarted;
        }
        if (t.isAlive())// precheck that t is startable
            throw new IllegalThreadStateException();
        workers.add(w);
        int s = workers.size();
        if (s > largestPoolSize)
            largestPoolSize = s;
        workerAdded = true;
    } finally {
        mainLock.unlock();
    }
    if (workerAdded) {
        t.start();
        workerStarted = true; }}finally {
    if(! workerStarted) addWorkerFailed(w); }return workerStarted;
Copy the code

Is the modified code easier to understand? If you add new functionality, you can change the code more easily.

conclusion

The JDK source code is functionally sound and architecturally sound, but I think it can be optimized for readability. The code for nested conditional expressions like this is not the only one in the JDK source code, either because the author didn’t think about using a guard statement at the time, or because he wasn’t as picky as I am. It is hoped that in the process of coding, you can also try to use the guard statement instead of nested conditional expression, in order to improve the clarity and maintainability of the code.

Reference: Refactoring: Improving the Design of Existing Code