Cleaner code: Inversion of logic

Design patterns

Almost all programmers have heard about and used design patterns. And there are a lot of them. Famous design patterns include the Singleton, Observer, Template method. These are all patterns that focus on objects and the relationship between them.

There are more groups of patterns, for example take Dependency Injection (or Inversion of Control). This is an architectural pattern. They tell you things about the design of the whole program. And there are specific pattern groups, like concurrency patterns.

These patterns help you solve common problems in a well defined way, other programmers will recognise the patterns used and it will help produce more maintainable code.

Inversion of logic

The patterns described above all say something about classes/methods and architecture. But I think another class of design patterns exist, which have a smaller granularity. These patterns say something about the code inside a single method. A good example is Inversion of logic.

Let me give an example, lets take the following code:

public void processRequests(List<RentalRequest> requests) {
	
	for (RentalRequest rentalRequest : requests) {
		if(rentalRequest.isFormFilledCorrectly()) {
			Tenant tenant = rentalRequest.getTenant();
			if(tenant.getAge() >= 25 && checkLicenceStillValid(tenant)) {
				//Approve the request
				//Process the request in database
				//Send email to HQ
				//Etc...
			}				
		}
	}
}

Its a basic car-rental service and is has some logic before a request is processed. To make this code more readable and maintainable we can do two things. Of course it would be better to factor out the validation-logic into another piece of code. So lets do that first:

public void processRequests(List<RentalRequest> requests) {
	
	for (RentalRequest rentalRequest : requests) {
    
		if(validateRequest(rentalRequest)) {
			//Approve the request
			//Process the request in database
			//Send email to HQ
			//Etc...
		}
	}
}

private boolean validateRequest(RentalRequest rentalRequest) {
	if(rentalRequest.isFormFilledCorrectly()) {
		Tenant tenant = rentalRequest.getTenant();
		if(tenant.getAge() >= 25 && checkLicenceStillValid(tenant)) {
			return true;
		}
	}
	return false;
}

Some programmers don’t like the validateRequest() method above because there are multiple exit points. This was more problematic in C code then in Java because in C you needed to do a bit more resource management (freeing mallocs etc), in Java I actually don’t mind having more then one exit-point in my code. In this case it makes it just a bit more readable. It would be easy to add a ‘requestIsValid’ boolean and fill that instead.

Anyway, the next step in refactoring this code is using what I call “Inversion of Logic”. The idea is that you don’t continue nesting if something is true, but instead inverse it, and bail out if something is false. The big advantage is that you’ll lose a lot of nesting, making the code more readable. I’m going to do this in two places, first in the processRequest for-loop, and second in the validateRequest method.

public void processRequests(List<RentalRequest> requests) {
	
	for (RentalRequest rentalRequest : requests) {
    
		if(!validateRequest(rentalRequest)) {
			continue;
		}
		
		//Approve the request
		//Process the request in database
		//Send email to HQ
		//Etc...
	}
}

private boolean validateRequest(RentalRequest rentalRequest) {
	if(!rentalRequest.isFormFilledCorrectly()) {
		return false;
	}
	Tenant tenant = rentalRequest.getTenant();
	if(tenant.getAge() < 25 || !tenant.checkLicenceStillValid(tenant)) {
		return false;
	}
	return true;
}

As you can see in the for-loop all the business logic is moved one indent back, this will make your code easier to scan for business rules. And if you look at the validateRequest method the code reads more fluent. Instead of stacking up and remembering the valid cases:

IF form is correct
AND tenant age correct
AND tenant licence is valid
THEN proceed

We now eliminate the bad cases first:

IF form is incorrect: deny rental
IF tenant’s age or licence is invalid: deny rental
ELSE proceed

I’m trying to do this everywhere I can, it realy helps the readability.