1. Introduction
In this article, I will discuss how to refactor the nested if conditions. Writing nested if conditions opens a whole slew of problems that we need to deal with. Not to mention that the code becomes increasingly cluttered and looks like an arrow.
Arrow code looks like below.
One thing we know, if we see nested if conditions in code, that code is begging to be refactored. Don’t let that opportunity go away.
If you don’t refactor, you may curse yourself in the future because anyone with access to this code can add more if conditions. Then the code becomes unmaintainable and haunts you because chances of bugs may creep up more and more.
2. Content
We will see 4 different ways to solve this interesting problem at hand. The 4 ways are :
- Eager Evaluation: Clubbing all conditions
- Lazy Evaluation: Flat if conditions and multiple return statements
- Lazy Evaluation: Clubbing all conditions
- Lazy Evaluation: Using Fluent API – Best resolution of all
More focus will be on Lazy Evaluation: Using Fluent API. Let us begin. Examples use JSR annotations for dependency injection in the Spring Boot.
3. Conditions
The data that we want to validate looks like this. The transaction request is a combination of transaction request from PayPal and Stripe. 🙂
{
"transactions": [{
"amount": {
"total": "30.11",
"currency": "USD",
"details": {
"subtotal": "30.00",
"tax": "0.07",
"shipping": "0.03",
"handling_fee": "1.00",
"shipping_discount": "-1.00",
"insurance": "0.01"
}
},
"description": "The payment transaction description.",
"payment_options": {
"card": {
"brand": "visa",
"country": "US",
"exp_month": 12,
"exp_year": 2020,
"fingerprint": "Xt5EWLLDS7FJjR1c",
"funding": "credit",
"installments": null,
"last4": "4242",
"network": "visa",
"three_d_secure": null,
"wallet": null
}
},
"soft_descriptor": "ECHI5786786",
"item_list": {
"items": [{
"name": "hat",
"description": "Brown hat.",
"quantity": "5",
"price": "3",
"tax": "0.01",
"sku": "1",
"currency": "USD"
},
{
"name": "handbag",
"description": "Black handbag.",
"quantity": "1",
"price": "15",
"tax": "0.02",
"sku": "product34",
"currency": "USD"
}
],
"shipping_address": {
"recipient_name": "Brian Robinson",
"line1": "4th Floor",
"line2": "Unit #34",
"city": "San Jose",
"country_code": "US",
"postal_code": "95131",
"phone": "011862212345678",
"state": "CA"
}
}
}]
}
3.1 3 validations to be written
- isTransactionRequestValid: Represents whether the transaction request that we got in request is valid or not. For this you could use javax validations along with hibernate validator implementation.
- isTransactionItemsValid: Represents whether transaction items are correct. For example: if the transaction amount currency is USD, then transaction items currency should also be USD, else the check will fail.
- isFundingMethodValid: Represents whether the payment_options provided in request is valid or not. For example: if the card’s expiry is in the past, then the check should fail. We cannot do this check in our service. There is a downstream service which does this check. The SLA for response is around < 150ms.
4. Eager Evaluation : Clubbing all conditions
The simplest solution to refactor the nested if conditions is to club them together in one if statement.
boolean isTransactionRequestValid = // validates transaction request
boolean isTransactionItemsValid = // validates transaction items
boolean isFundingMethodValid = // validates funding method
if(isTransactionRequestValid
&& isTransactionItemsValid
&& isFundingMethodValid) {
// do something
}
This is a really good solution. Isn’t it? We can just club all conditions together and be done with it.
Now let me ask you an interesting question: If we club all 3 conditions we will make a call to outbound service regardless of what response is for condition1 or condition2. Is it really worth it to make checks for condition2 and condition3 if condition1 is false? No it is not. We are doing extra work even though it is not needed.
Below is how the code would look like :
@Named
@Scope("request")
public class TransactionResourceImpl {
private final TransactionRequestValidator requestValidator;
private final TransactionItemsAmountValidator itemsValidator;
private final FundingMethodValidator fundingMethodValidator;
private final TransactionProcessor processor;
@Inject
public TransactionResourceImpl(
TransactionRequestValidator requestValidator,
TransactionItemsAmountValidator itemsValidator,
FundingMethodValidator fundingMethodValidator,
TransactionProcessor transactionProcessor) {
this.requestValidator = requestValidator;
this.itemsValidator = itemsValidator;
this.fundingMethodValidator = fundingMethodValidator;
this.processor = transactionProcessor;
}
public TransactionResponse process(TransactionRequest request) {
TransactionResponse response = null;
if (requestValidator.validate(request)) {
if (itemsValidator.validate(request)) {
if (fundingMethodValidator.validate(request)) {
response = processor.processPayment(request);
}
}
}
if(response == null) {
throw new IllegalArgumentException();
}
return response;
}
}
What we did just right now was eager evaluation of validators. What we need to do is to not execute those validators if they are not needed at all. For example : if the request is null then the first condition would fail then there is no point in executing condition2 and condtion3.
My vote is not to use this approach.
5. Lazy Evaluation : Flat if conditions and multiple return statements
One naive way to refactor nested if conditions would be like this.
public TransactionResponse process(TransactionRequest request) {
boolean validate = requestValidator.validate(request);
if (!validate) {
throw new IllegalArgumentException();
}
boolean validate2 = itemsValidator.validate(request);
if (!validate2) {
throw new IllegalArgumentException();
}
boolean validate3 = fundingMethodValidator.validate(request);
if (!validate3) {
throw new IllegalArgumentException();
}
return processor.processPayment(request);
}
If the validation fails then throw an exception else move ahead with other validations. What is the problem with this approach, you may ask? The problem is multiple return statements and too many if conditions in one method.
My vote is not to use this approach.
6. Lazy Evaluation : Clubbing all conditions
In the previous solution we just clubbed all the validation results. So all the validations are executed first and then we club its result.
In this approach we can defer the execution of validators until they are needed. How to do that? Just use a functional interface.
The process needs to be changed as below :
public TransactionResponse process(
TransactionRequest request) {
Predicate<TransactionRequest> p1 =
(req) -> requestValidator.validate(req);
Predicate<TransactionRequest> p2 =
(req) -> itemsValidator.validate(req);
Predicate<TransactionRequest> p3 =
(req) -> fundingMethodValidator.validate(req);
boolean shouldProcess = p1.test(request)
&& p2.test(request)
&& p3.test(request);
if (!shouldProcess) {
throw new IllegalArgumentException();
}
return processor.processPayment(request);
}
So if the first Predicate returns false other Predicates won’t be executed at all. This is a good solution. But we can still do better. You can remove the if logic from this implementation and make this code extremely clear using FLuent API.
My vote is not to use this approach.
7. Lazy Evaluation : Using Fluent API
This is by far the best approach. Read on, you will know why I said that.
In this approach we won’t use functional interfaces but still the conditions will be evaluated lazily.
A Fluent API or Fluent Interface is an object oriented API which relies excessively on method chaining. For example : Stream<T> interface in Java 8.
List<String> result = list .stream() .filter(pred) .map(func) .collect(Collectors.toList());
Our final code will look like this :
public TransactionResponse process(
TransactionRequest transactionRequest) {
transactionValidation
.validateRequest(transactionRequest)
.validateTxnItems(transactionRequest)
.validateFundingMethod(transactionRequest)
.throwIfFailure(IllegalArgumentException::new);
return processor.processPayment(transactionRequest);
}
Let us design a Fluent API using a class called TransactionValidation. There are 3 pieces to it :
- Maintains validation state
- Has validation delegators
- Exception throwing mechanism
Let us understand these parts in detail.
7.1 Maintains validation state
In order to maintain the state for any object we need to have a proper scope for this class.
- Singleton won’t work as we must not maintain state in singleton class.
- Request shouldn’t be used as we are not dependent on anything that comes in request or request headers.
- The best scope to maintain state per instance is prototype scope.
@Named
@Scope("prototype")
public class TransactionValidation {
private boolean validationState;
}
And that should be it. Moving on.
7.2 Has validation delegators
Just inject the validators that you need in this class. Use constructor injection to do so. Please don’t use field or setter injection.
@Named
@Scope("prototype")
public class TransactionValidation {
private final TransactionRequestValidator requestValidator;
private final TransactionItemsAmountValidator itemsValidator;
private final FundingMethodValidator fundingMethodValidator;
private boolean validationState;
@Inject
public TransactionValidation(
TransactionRequestValidator requestValidator,
TransactionItemsAmountValidator itemsValidator,
FundingMethodValidator fundingMethodValidator) {
this.requestValidator = requestValidator;
this.itemsValidator = itemsValidator;
this.fundingMethodValidator = fundingMethodValidator;
}
}
Now comes the fun part. Writing the Fluent API.
The first thing we want is to execute base validator i.e. request validator. We can write code like this.
public TransactionValidation validateRequest(
TransactionRequest request) {
validationState = transactionRequestValidator.validate(request);
return this;
}
We save the state of the request validator in variable validationState and we return “this”. this refers to the current object of this class the method is being operated upon. So now you have the ability to chain multiple methods of the same class.
Now let us take this idea to the next level. Remember the idea here is to not execute validations unnecessarily.
Write a transaction items validator. This validator will only be executed if the previous validator passed else no operation is done. Same instance is returned.
public TransactionValidation validateTxnItems(
TransactionRequest request) {
if (validationState) {
validationState = itemsValidator.validate(request);
}
return this;
}
Same with funding method validator.
public TransactionValidation validateFundingMethod(
TransactionRequest request) {
if (validationState) {
validationState = fundingMethodValidator.validate(request);
}
return this;
}
Excellent. Now let us use this class and its methods.
transactionValidation .validateRequest(transactionRequest) .validateTxnItems(transactionRequest) .validateFundingMethod(transactionRequest);
The best part about this API design is that the code is so easy to understand that anyone can use it with great ease.
There is one problem though. As we are returning “this” we need to terminate the chaining. For that we will just throw an exception if validation fails. Let us look at how to throw an exception.
7.3 Exception throwing mechanism
Throwing an exception is simple. Just throw an exception if the validation has failed. I have copied this method from Optional class’s orElseThrow method.
public <X extends Throwable> void throwIfFailure(
Supplier<? extends X> exceptionSupplier) throws X {
if (!validationState) {
throw exceptionSupplier.get();
}
}
Client can use this method like this :
transactionValidation .validateRequest(transactionRequest) .validateTxnItems(transactionRequest) .validateFundingMethod(transactionRequest) .throwIfFailure(IllegalArgumentException::new);
As simple as that. Clean, elegant, easy to understand and easy to test code.
This is how entire class would look like :
@Named
@Scope("prototype")
public class TransactionValidation {
private final TransactionRequestValidator requestValidator;
private final TransactionItemsAmountValidator itemsValidator;
private final FundingMethodValidator fundingMethodValidator;
private boolean validationState;
@Inject
public TransactionValidation(
TransactionRequestValidator requestValidator,
TransactionItemsAmountValidator itemsValidator,
FundingMethodValidator fundingMethodValidator) {
this.transactionRequestValidator = requestValidator;
this.itemsValidator = itemsValidator;
this.fundingMethodValidator = fundingMethodValidator;
}
public TransactionValidation validateRequest(
TransactionRequest request) {
validationState = request.validate(transactionRequest);
return this;
}
public TransactionValidation validateTxnItems(
TransactionRequest request) {
if (validationState) {
validationState = itemsValidator.validate(request);
}
return this;
}
public TransactionValidation validateFundingMethod(
TransactionRequest request) {
if (validationState) {
validationState = fundingMethodValidator.validate(request);
}
return this;
}
public <X extends Throwable> void throwIfFailure(
Supplier<? extends X> exceptionSupplier) throws X {
if (!validationState) {
throw exceptionSupplier.get();
}
}
}
The client code looks like this :
@Named
@Scope("request")
public class TransactionResourceImpl {
private final TransactionProcessor processor;
private final TransactionValidation transactionValidation;
@Inject
public TransactionResourceImpl(
TransactionProcessor processor,
TransactionValidation transactionValidation) {
this.processor = processor;
this.transactionValidation = transactionValidation;
}
public TransactionResponse process(TransactionRequest request) {
transactionValidation
.validateRequest(request)
.validateTxnItems(request)
.validateFundingMethod(request)
.throwIfFailure(IllegalArgumentException::new);
return processor.processPayment(request);
}
}
8. Conclusion
In this article we saw how to refactor nested if conditions using 4 different ways. I would recommend you to use the Fluent API method to resolve it. Using Fluent API makes your code clean, simple, elegant, easier to understand and easier to test.