patternjavaMinor
Handling null arguments in a factory class
Viewed 0 times
handlingargumentsfactorynullclass
Problem
I have a
So, instead of asking calling program to pass
Could you let me know how to make the following piece of code better?
Factory class that gives out instances of ResponseImpl class. It accepts one Destination class and up to four Source classes. At least one of the four Source classes should be not null.So, instead of asking calling program to pass
nulls in the arguments to Factory class, I have overloaded the method to accept at least one up to four Source classes. But, something tells me that this way of doing it is not a good idea. Although this works, I feel like this can be handled better.Could you let me know how to make the following piece of code better?
public static Response initResponse(final Destination destination, final Source1 source1) {
return initResponse(destination, source1, null, null, null);
}
public static Response initResponse(final Destination destination, final Source1 source1, final Source2 source2) {
return initResponse(destination, source1, source2, null, null);
}
public static Response initResponse(final Destination destination, final Source1 source1, final Source2 source2, final Source3 source3) {
return initResponse(destination, source1, source2, source3, null);
}
public static Response initResponse(final Destination destination, final Source1 source1, final Source2 source2, final Source3 source3, final Source4 source4) {
try {
if(source1 == null && source2 == null && source3 == null && source4 == null) {
throw new ABODataException("Atleast one source has to be not null");
}
return ResponseImplManager.getInstance(destination, source1, source2, source3, source4);
} catch (Exception e) {
throw new ABODataException("Unable to instantiate Response:"+e.getMessage());
}
}Solution
It seems to me that you are not quite making good use of exceptions and as long as I believe it is possible that you are disrespecting some principles. First things first, you really should be using
IllegalArgumentException because that is the java standard exception class for invalid arguments. You shouldn't also throw a exception and then catch it so your throw statement should be outside the try block. Why should you ever raise a error to treat it imeadiatly? You should avoid it instead!public static Response initResponse(final Destination destination, final Source1 source1, final Source2 source2, final Source3 source3, final Source4 source4) {
if(source1 == null && source2 == null && source3 == null && source4 == null) {
throw new IllegalArgumentException ("Atleast one source has to be not null");
}
try {
return ResponseImplManager.getInstance(destination, source1, source2, source3, source4);
} catch (Exception e) {//if possible catch the specific type of the exception that is thrown by getInstance thought I don't see it as a huge issue as you don't have any other method calls
throw new ABODataException("Unable to instantiate Response:"+e.getMessage());
}
}Code Snippets
public static Response initResponse(final Destination destination, final Source1 source1, final Source2 source2, final Source3 source3, final Source4 source4) {
if(source1 == null && source2 == null && source3 == null && source4 == null) {
throw new IllegalArgumentException ("Atleast one source has to be not null");
}
try {
return ResponseImplManager.getInstance(destination, source1, source2, source3, source4);
} catch (Exception e) {//if possible catch the specific type of the exception that is thrown by getInstance thought I don't see it as a huge issue as you don't have any other method calls
throw new ABODataException("Unable to instantiate Response:"+e.getMessage());
}
}Context
StackExchange Code Review Q#40559, answer score: 7
Revisions (0)
No revisions yet.