Design Question : How can I refactor a validation check into another method
I have a huge method of validate that I want to refactor. Here is an example. I have provided the initial validation, and my own suggestion. Do you think i开发者_如何学Got's a good way to refactor it or there are better ways to refactor this?
Thanks,
public void validate() {
MyException myException= new MyException();
try {
if (!isRule1()) {
throw MyException.invalidRule(Rule1);
}
}
catch (MyException e) {
myException.addWrappedException(e);
}
try{
f (!isRule2()) {
throw MyException.invalidRule(Rule2);
}
}
catch (MyException e) {
myException.addWrappedException(e);
}
////continue checking...
}
and here is how I want to refactor them:
public void validate() {
MyException myException= new MyException();
validateRule1(myException);
validateRule2(myException);
//continue validating....
}
private void validateRule1(myException){
try {
if (!isRule1()) {
throw MyException.invalidRule(Rule1);
}
}
catch (MyException e) {
myException.addWrappedException(e);
}
}
private void validateRule2(myException){
try {
if (!isRule2()) {
throw MyException.invalidRule(Rule2);
}
}
catch (MyException e) {
myException.addWrappedException(e);
}
}
You could use the Decorator pattern here. Create an interface that has a single method validate() and then create different classes implementing this interface containing the validation code. Then you can call validate on the object you build and this will unwind through all the other validations.
This has the added benefit of improving the modularity of the code, enabling you to unit test the validations independently. You can also easily add or remove validations from the chain as needed.
A simple example is coded below.
public interface Validator {
void validate(Object input) throws ValidationException
}
public class ValidationOne implements Validator {
protected Validator validator;
public ValidationOne(Validator validator) {
this.validator = validator;
}
public void validate(Object input) throws ValidationException {
if (validator != null)
validator.validate(input);
// do specific ValidationOne checks
if (!isValid(input)) throw new ValidationException()
}
}
public class ValidationTwo implements Validator {
protected Validator validator;
public ValidationTwo(Validator validator) {
this.validator = validator;
}
public void validate(Object input) throws ValidationException {
if (validator != null)
validator.validate(input);
// do specific ValidationTwo checks
if (!isValid(input)) throw new ValidationException()
}
}
public class Tester {
public void runValidations(Object obj) {
Validator validator = new ValidationOne(null);
validator = new ValidationTwo(validator);
// continue adding validations as needed
try {
validator.validate(obj);
} catch (ValidationException e) {
System.err.println("Validation error occurred");
}
}
}
What about trying
MyException myException= new MyException();
if (!isRule1())
myException.addWrappedException(MyException.invalidRule(Rule1));
or
MyException myException= new MyException();
myException.addWrappedException(checkRule1());
where checkRule1() returns an exception or null and addWrappedException ignores nulls.
or
MyException myException= new MyException();
checkRule1(myException);
Definitely start with Replace Method with Method Object
. It's not risky and you shouldn't be afraid of it with such a huge method. Then your approach seems to be good... just be sure to rename things like validateRule1
;)
精彩评论