Java, return a new MyException: anti-pattern?
In my class I am doing user data validation. Many conditions apply. On any failure, I want to throw a specific MyException. Throwing this MyException takes many general parameters and one configurable parameter (based on the actual failure). So the actual cast requires a lot of characters to write and destroy neatness due to code duplication. Also I have to give up too much time. I decided to create a private method that prepares and returns a new instance of this MyException and only accepts user data as a parameter, so the code could be much cleaner.
private MyException createMyException(final CustomErrorData errorData)
{
... some info gathering, parameterizing, etc...
return new MyException(errorData);
}
...
So, throwing a new MyException is much shorter:
throw createMyException(errorData);
My question is, what is the correct practice to avoid code duplication in this case? I can persuade Exceptions.
source to share
Factory exception - never seen it before, but at least it sounds like correct design.
I'm just worried - you seem to be putting a lot of effort into developing an exception framework: adding parameters, states, etc. to exceptions. Are you really running into a lot of exceptional conditions in your code? Or do you throw exceptions when the correct handling of the expected conditions will be?
A common exception thrown is "log only". Something happened that shouldn't be in the current context. Something developers need to know and fix in the next version. We shouldn't use exceptions to handle expected states.
So, before exploring the flamboyant exception throwing code, double check to see if it's worth doing, or your app's design starts ... too creative.
source to share
If you have one general type of exception, you will lose some of the benefits of OOP.
Instead of being able to try-catch for certain types of exceptions, you will have to have a catch for your general exception and then continue processing based on some fields inside your MyException class.
You will have something like this:
try{
//code here
}
catch (MyException ex){
switch(ex.exceptionType){
case IOException: doSomething();break;
case ConnectionException:doSomethingElse();break;
default: //throw the exception outwards if you don't want to process it
}
}
If instead you should have something like
try{
//code here
}
catch (IOException ex){
doSomething();
}
catch (ConnectionException ex){
doSomethingElse();
}
which is more clear and more OOP.
Why would you put all your exceptions under a generic type, it’s a bit of a puzzle, how to make all your objects instances of only one class, but you would require them to do different things based on some flags.
source to share
I would throw an exception in the method if it doesn't confuse the compiler.
private void throwMyException(final CustomErrorData errorData) {
... some info gathering, parameterizing, etc...
throw new MyException(errorData);
}
throwMyException(errorData);
or
private MyException throwMyException(final CustomErrorData errorData) {
... some info gathering, parameterizing, etc...
throw new MyException(errorData);
}
throwMyException(errorData);
// or if the compiler complains
throw throwMyException(errorData);
source to share
I would separate two issues. Your class knows how to collect information, but does not need to know about the exception (the user of this information).
First, define the instantiation method CustomErrorData
:
private CustomErrorData createCustomErrorData() {
// info gathering
return new CustomErrorData(something);
}
Then define a constructor for the exception, which uses CustomErrorData
:
public MyException(CustomErrorData errorData) {
// save it as a field
}
then
throw new MyException(createCustomErrorData());
where you need it.
It also allows it to be used CustomErrorData
for something else, perhaps for logging in, for displaying to the user, whatever.
source to share