Reputation: 1670
Say I have a Utility class. In this class, I am exposing only 2 public functions:
public static boolean checkCustomerEligiblity(HttpServletRequest request)
public static boolean checkCartEligiblity(HttpServletRequest request)
Each of these method’s implementation is really messy (I did not implement this method). For future reference I want to understand the best way we can implement in such scenarios.
Point to also remember is that in cases of FALSE conditions we are not supposed to exit or return. We have to record the FALSE condition with a reason and proceed with the rest of the checks.
public static Boolean checkCustomerEligiblity(HttpServletRequest request) {
if (Flags) { //Check if Flags are ON, only then proceed with rest
if (Type of customer) { //Restrict only to certain type of customers
if (Customer within limit) { //Check customer’s available cash limit in account
} else reasons.add(“Customer not within limit”);
} else reasons.add(“Customer type not supported”);
if (Customer account is NULL) {…}
if (Customer skipped verification with photo ID) {…}
if (Customer’s organization has opted in the plan) {…}
if (Customer’s bill is combined bill) {…}
if (Customer has past due on his account) {…}
if (Customer’s credit rating is good) {…}
if (Customer’s account is active) {…}
if (Customer has not opted for this plan already) {…}
...around 15-20 If conditions more...
}
}
The same structure goes for the checkCartEligibility() method.
My question is –
1) Will it be too unwieldy to use Strategy or Command design pattern to implement the above?
For instance,
public interface Validator {
Boolean validate(HttpServletRequest);
}
public class CustomerCreditValidator implements Validator {…}
public class FlagValidator implements Validator {…}
public class AccountVerificationValidator implements Validator {…}
public class OrderTypeValidator implements Validator {…}
public class CartValidator implements Validator {…}
…so on with around 10-15 more validator classes (I can combine couple of checks in the same class to lower down the number of such classes).
List<Validator> validators = new ArrayList<>();
validators.add(new CustomerCreditValidator());
validators.add(new FlagValidator());
validators.add(new AccountVerificationValidator());
validators.add(new OrderTypeValidator());
validators.add(new CartValidator());`
…so on for other classes.
for (Validator validator : validators) {
boolean result = validator.validate(request);
if (result) {
...
}
2) If the above approach is going to be too cumbersome too, what is/are the other design pattern(s) would you propose the implement the above logic?
3) By the way, each of the validation checks are private, so can I have all the above validator classes as inner classes only?
Much appreciate any help!
Upvotes: 4
Views: 8472
Reputation: 165
You can look at spring state machine. We have used extensively in our project that involves multiple steps with each steps having it's own set of validations.
Upvotes: 1
Reputation: 1670
I will go out on a limb and propose – Build Pattern – for such scenario. Please hear me out.
For instance,
Let’s have a class:
public class BaseValidator {
private final List<CustomException> exceptions;
private BaseValidator(final Validator validator) {
exceptions = new ArrayList<CustomException>(validator.exceptionsAll);
}
public boolean hasException() {
return exceptions.isEmpty();
}
public List<CustomException> getAllExceptions() {
return exceptions;
}
public void throwFirstExceptionIfAny() throws CustomException {
if (!exceptions.isEmpty()) {
throw exceptions.get(0);
}
}
public static class Validator {
private final List<CustomException> exceptionsAll = new ArrayList<CustomException>();
public Validator flag(String FLAG_NAME) {
if (request.getSession().getConfigurables().getFlag(FLAG_NAME))
...
else exceptionsAll.add(new CustomException(“Flag is OFF”));
}
public Validator credit(long CreditScore) {
if (CreditScore > 100) {
...
} else exceptionsAll.add(new CustomException(“Credit score less than required”));
return this;
}
public Validator account(CustomerAccount account) {
//Do account verification, else add new CustomeException() to exceptionsAll
return this;
}
public Validator ordertype(String orderType) {
...
return this;
}
public Validator agreement(Boolean hasDeclinedAgreement) {
...
return this;
}
…so on for other condition checks
public BaseValidator validate() {
return new BaseValidator(this);
}
}
Now we can use it like this. Let’s say I have to validate the Customer:
BaseValidator customerValidation =
new Validator()
.flag(getConfigurables().getFlag()).
.account(request.getSession().getAccount())
.agreement(request.getSession().hasDeclinedAgreement())
.validate();
If (customerValidation.hasException())
List<CustomException> all = customerValidation.getAllExceptions();
If I have to validate the Cart, I can call different validator methods:
BaseValidator cartValidation = new Validator()
.flag(getConfigurables().getFlag())
.ordertype(…)
…
…
.validate();
cartValidation.throwFirstExceptionIfAny();
I know we do not traditionally use Builder Pattern for validation purposes but can we use it this way to clean up?
Upvotes: 1
Reputation: 131346
As John Bollinger, I don't think also that multiplying the number of classes will make your conditional logic more clean.
Here where you can improve things is the use of helper methods to handle each type of way of checking.
For example these conditions could be extracted in common methods which validate and add the reason to reasons
if the criteria matches :
For example :
Instead of
if (!Customer within limit) {
reasons.add(“Customer not within limit”);
}
You could call :
validCustomerInLimit(reasons, customer);
Instead of
if (Customer account is NULL)
reasons.add(“Customer account is null”);
}
You could call :
validCustomerAccountNotNull(reasons, customer);
An still more efficient implementation would be to have a dedicated class you instantiate for each validation you perform.
In this way, reasons
and customer could be instance fields and you could directly call the helper methods without providing reasons
or customer
as parameter.
You could directly do it :
ValidatorCustomer validatorCustomer = new ValidatorCustomer(request);
validatorCustomer.validAccountInLimit();
validatorCustomer.validAccountNotNull();
Upvotes: 1
Reputation: 180201
Design patterns such as Strategy and Command are aimed at solving a problem in terms of loosely-coupled components. This can be advantageous for code reuse, and it may be advantageous for a design on which new features can readily be added or existing ones readily modified. These patterns are not specially useful for general organization of code for which such needs are not anticipated, however.
Will it be too unwieldy to use Strategy or Command design pattern to implement the above?
It does seem like that would make for rather a lot more code than you have now, and certainly more separate classes, and some glue code that you don't need at all right now. Supposing that all the new classes would be single-use, as seems likely to be the case, I can't say I see any allure.
If the above approach is going to be too cumbersome too, what is/are the other design pattern(s) would you propose the implement the above logic?
I don't think this situation necessarily calls for a named pattern so much as simply for better style. In particular, I'd consider writing each individual check as a separate, private method, without wrapping that up in its own class. The public checker methods would then consist mostly or even wholly of a sequence of invocations of the individual check methods.
By the way, each of the validation checks are private, so can I have all the above validator classes as inner classes only?
If you did go with validator classes then you could make those classes private nested classes, yes, whether inner classes or static nested ones. But the situation you now posit even more suggests to me that such an approach would be overkill. Try first just factoring it into multiple methods.
Upvotes: 5