CODEWITHSUNDEEP
javaexception
gyorgyabraham
gyorgyabraham

Reputation: 2608

Java, return new MyException: anti-pattern?

In my class I'm doing validation of custom data. Many conditions apply. Upon any failure, I want to throw a specific MyException. Throwing this MyException takes many common parameters, and one custom parameter (based upon the actual failure). So an actual throw takes many characters to write and destroys tidyness because of code duplication. Also I have to throw it too much times. I made up my mind to create a private method that prepares and returns a new instance of this MyException and takes the only custom data as parameter, so the code can 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's the correct practice to prevent code duplication in this case? I may be overmistifying Exceptions.

Upvotes: 3

Views: 258

Answers (5)

Bohemian
Bohemian

Reputation: 425278

I'd separate the two concerns. Your class knows how to info gather, but shouldn't have to know about the exception (the user of that info).

First define a method to create a CustomErrorData instance:

private CustomErrorData createCustomErrorData() {
    // info gathering
    return new CustomErrorData(something);
}

Then define a constructor for the exception that uses a CustomErrorData:

public MyException(CustomErrorData errorData) {
    // save it as a field
}

then

throw new MyException(createCustomErrorData());

where you need it.

This also allows you to use CustomErrorData for something else, perhaps logging, displaying to the user, whatever.

Upvotes: 0

Andreas Dolk
Andreas Dolk

Reputation: 114817

An Exception factory - never seen it before but at least it sounds like a proper design.

I just worry - you seem to put quite a lot effort on designing an exception throwing framework: adding parameters, states, etc. to exceptions. Do you really encounter that many exceptional conditions in your code? Or do you throw exceptions where proper handling of expected conditions would?

Usually a thrown exception is "just for the logs". Something happened that shouldn't have happened in the current context. Something, the developers should know and correct in the next release. We shouldn't use exceptions to handle expected states.

So before investigating in brilliant exception creation code, double-check if it's worth the effort or if the design of your application is starting to get ... too creative.

Upvotes: 2

Andrei G
Andrei G

Reputation: 1590

If you have one general type of exception you will lose some of the advantages of OOP.

Instead of being able to have try-catch for specific exception types 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
 }
}

When 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 you would place all your exceptions under a general type is something of a puzzle, it's like making all your objects to be instances of only one class, but you would require of them different behaviors based on some flags.

Upvotes: 1

Peter Lawrey
Peter Lawrey

Reputation: 533780

I would throw the exception in the method, unless this confuses 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);

Upvotes: 0

quaylar
quaylar

Reputation: 2635

Imho your helper function is perfectly fine, i dont see another approach that would be preferable here..

Upvotes: 0

Related Questions