I am refactoring some legacy code and I come across this function:
private static void parseOptionalValues(Product product, Input source) {
try {
product.setProductType(...some operation with source...);
} catch (IllegalArgumentException ignored) {}
try {
product.setMaterial(...some operation with source...);
} catch (IllegalArgumentException ignored) {}
try {
product.setUnitPricingBaseMeasure(...some operation with source...);
} catch (IllegalArgumentException ignored) {}
try {
product.setUnitPricingMeasure(...some operation with source...);
} catch(IllegalArgumentException ignored){}
}
My common sense says to me that in order to keep the Don't Repeat Yourself principle I should wrap this try-catch logic so I introduced this change:
private static void parseOptionalValues(Product product, Input source) {
setOptionalParameter(...some operation with source..., product::setProductType);
setOptionalParameter(...some operation with source..., product::setMaterial);
setOptionalParameter(...some operation with source..., product::setUnitPricingBaseMeasure);
setOptionalParameter(...some operation with source..., product::setUnitPricingMeasure);
}
private static <T> void setOptionalParameter(T value, Consumer<T> consumer) {
try {
consumer.accept(value);
} catch (IllegalArgumentException ignored) {}
}
I am running some unit tests and debugging but the code doesn't behave as previously as the IllegalArgumentException is not catched but raised so the program fails.
Any idea on how to solve this enclosing the try-catch logic in a single place?
I think the problem might be that the exception is thrown by the code that gets the arguments to be passed to the setters. So one possible approach would be to make that part of the code lazy by using a Supplier, then invoke .get() on the Supplier inside a the try/catch block (as well as invoking the Consumer):
private static <T> void setOptionalParameter(
Supplier<? extends T> supplier,
Consumer<? super T> consumer) {
try {
consumer.accept(supplier.get());
} catch (IllegalArgumentException ignored) {
}
}
You could invoke that method as follows:
setOptionalParameter(() -> ...some operation with source..., product::setProductType);
Note that I've improved the signature of your method so that it now accepts a wider range of generic subtypes and supertypes for the Supplier and Consumer, respectively.
EDIT: As per the comments, the approach above might be not flexible enough, i.e. if the method accepts more than one argument, etc. In this case, it would be better to use a Runnable instance:
private static void setOptionalParameter(Runnable action) {
try {
action.run();
} catch (IllegalArgumentException ignored) {
}
}
And invocation would now become:
setOptionalParameter(() -> {
ProductType productType = ...some operation with source...;
Material material = ...some operation with source...;
product.doSomethingWith2Args(productType, material);
});
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With