0

Say I have a hypothetical factory method whose single responsibility is to create MyObjects. However, MyObject should only ever be constructed with an ordered list. Further, MyObjects without an ordered list should never exist my world.

Is it the responsibility of the factory to make sure the list is ordered before creating a new MyObject?

public class Factory {
    public MyObject make(List<Integer> myList) {
        // Should I check if the myList is ordered here?
        if (myList is not ordered) {
            throw new IllegalArgumentException("List not ordered");
        }
        return new MyObject(myList);
    }
}

Or is it the responsibility of the Object itself to make sure the list is ordered in the constructor as shown below?

public class MyObject {
    public MyObject(List<Integer> myList) {
        // Or should I check if myList is ordered here?
        if (myList is not ordered) {
            throw new IllegalArgumentException("List is not ordered");
        }
    }
}

My own arguments in favour of option 1:

  • Since the factory's sole job is to create MyObjects, it feels somewhat reasonable that this validation logic should be the responsibility of the thing that is making the object.

My own arguments in favour of option 2:

  • By keeping validation logic in MyObject's constructor, we increase the Cohesion of our code (from GRASP), as this validation logic is directly relevant to MyObject itself.
  • We also improve modularity of our MyObject class since it can be easily re-used in other areas of our code. For example, if we went with option 1, we would need to duplicate the validation logic before constructing a MyObject directly (if we decided for whatever reason we didn't want to use our Factory from option 1).

My own opinion is that Option 2 is probably the more preferred approach in this specific instance. What are people's thoughts?

3
  • 1
    The design pattern called factory method is a polymorphic method, so you probably mean a simple factory eg sourcecodeexamples.net/2017/12/factory-design-pattern.html – Fuhrmanator Jul 24 at 3:22
  • 1
    Also, ordered and sorted mean different things. Lists in Java are ordered meaning one has control over the order. I'm assuming that you mean sorted. – Fuhrmanator Jul 24 at 3:32
  • Yep, good point about "factory method". I was trying to say "method in a factory" by saying "factory method". I did make a conscious effort not to say the "factory method pattern", which my crude example definitely is not. Also, agreed - sorted is probably more precise. – p4t Jul 24 at 3:37
5

One more aspect:

Your MyObject constructor is public, so any piece of code might create MyObject instances. In this situation, you cannot rely on just a validation in the Factory.

As long as you can't guarantee that the only place able to create MyObject instances is the Factory, you have to validate in the constructor.

Then, is there any reason why an additional check in the Factory might be useful? Maybe if the Factory check can be done early, and avoid costly steps later, before calling the constructor. But even that argument isn't convincing, as invalid arguments seem to be the exception in your software (you literally throw exceptions in such cases). And reasoning about performance in exceptional cases is mostly nonsense.

4

If the responsibility of the factory is to create some kind of objects, it is also its responsibility to ensure that they are created correctly. So go for option 1.

If your worry is about the single responsibility, keep in mind that the SRP is about reasons to change, and the factory can only have on reason to change: if the construction rules for the object change. So SRP is a non-issue here.

An exception in a constructor should in principle be exceptional and unavoidable, i.e construction cannot be completed or object would not be usable. This technique should be used for avoiding inconsistencies, i.e. if an unordered list would at some moment lead the object into an unstable/inconsistent state. In this case option 2 would be a relevant defensive programming technique (“belt and braces”). If this is just to ensure some arbitrary check on application data, and technically MyObject could work with an unordered list, this would be another responsibility, and could reduce possibility of reuse of the class in another context.

The very important point for DRY is to avoid doing the same check at two places. So more important than the choice between 1 and 2 is that you stay consistent with your (your team’s?) general approach.

1
  • Thanks so much for sharing your insights - I found it incredibly useful. I also read through your SRP link from Robert Martin and it felt like a breath of fresh air to hear about it from the horse's mouth! I also love your "belt and braces" analogy for defensive programming haha (first time hearing it). – p4t Jul 22 at 11:16
-1

Does the class modify the list? Why not just make a sorted copy?


The poster asked a specific question, I gave a specific answer. For the general question: validate no higher than the lowest public method. If the constructor is public, validate there. If the constructor is private, either is fine.

3
  • You raise a good point here. I guess the example I've given is quite contrived and the condition of "needs to be sorted" is overly simplistic. However, I think the over-arching question I'm asking is - if I have some complex validation logic that needs to be applied to the arguments of a constructor, does it belong in the factory or in the constructor? I guess if we were to stick with the sorting analogy and your suggestion to just make a sorted copy. The question is, is it better design to provide a sorted list from the factory or is it better deesign to make a sorted copy in the constructor? – p4t Jul 22 at 3:05
  • 1
    The more I think about this, the more I feel like my question is a non-question and that the logic should be in the constructor... maybe I'm just overthinking it. Curious to hear opinions anyways. Maybe this question is a sign of a hidden code smell. – p4t Jul 22 at 3:08
  • This is definitively how I would proceed…. if there would be an absolute order on the things in the list, and if this would not lead to multiple sorting requests on the same list in the call chain (and to avoid this I’d remove the requirement for a sorted list as constructor parameter) – Christophe Jul 22 at 11:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.