Handling null arguments in a factory class

Question

I have a 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());
 }
}

Answer 1

It's a bit problematic to refactor when the parameters are from four different types of classes. I have a couple of alternative approaches for you though, pick one if you like one.

One option would be to make your classes Source1, Source2, etc. implement one common interface. Perhaps this would even simplify your ResponseImplManager.getInstance? If they would be of the same interface (or shared superclass would also be possible of course), then you could use a method like this:

public static Response initResponse(final Destination destination,
 final SourceInterface... sources) {
 // sources is treated as an array here
}

Or you could send a list of sources

public static Response initResponse(final Destination destination,
 final List<SourceInterface> sources)

Another option would be to use a Builder pattern

Whichever way you go in, I got one more suggestion. Use the two-parameter constructor for Exceptions to provide "Caused by" information, this will give you a more detailed stack trace

 } catch (Exception e) {
 throw new ABODataException("Unable to instantiate Response", e);
 // or use this:
 throw new ABODataException("Unable to instantiate Response: " + e.getMessage(), e);
 }

• Bruno Costa: I was going to add a edit about using variable arguments too but then I noticed that the classes were not the same and then reached the same conclusion as your. a big +1 for you.
• would_like_to_be_anon: Looks like using Builder pattern is the way to go. I didn't think about that.
• Simon Forsberg: @would_like_to_be_anon That's what you have us for, to think for you after you've done the initial thinking :)

Answer 2

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());
 }
}