Campbell Ritchie wrote:Welcome again
I am not happy about your unmodifiable list call there. I think unmodifiable list is suitable for returning a List but not so good for receiving a List. When you return a List, you have four options (I think four):-
1: Return the List full stop. That means you have two references to it and changes in one place are immediately reflected in the other place. There can be concurrency problems because most Lists aren't thread‑safe. 2: Return an unmodifiable List. That means changes in the original location are reflected in both places but it is impossible to change the List from its destination. If you try to alter the List structurally will cause an exception to be thrown. The destination receives a read‑only copy. 3: Return a copy of the List. That means both source and destination have a List as it was at the time of method completion, but they are independent. Both can be changed, but those changes are not reflected in the other. 4: Copy the List and return it in unmodifiable form. That means that the source can change the List without the destination's List changing at all, but the destination cannot alter the List.Note none of those things stops anybody from taking a fresh copy of the List and modifying that. Nor does anything make any of the elements of the List immutable: you can always change the state of a mutable object.Maybe want to take a copy of that List, so any changes you make to it will be independent of the original. Do the copying before you do the validation.
No, copying should precede validation because there are security hazards if validation is done first. It is possible to change the state of an argument after validation and before copying to include naughty code.What sort of error are you getting? If you are getting a null pointer exception, there is nothing wrong with that. It isn't your problem, but the problem of whoever is passing the null reference. The requireNonNull call will produce such an exception in advance. Now all you need to do is verify that the List isn't empty in your verification method. The old version was quite all right.
Sylvia Rosemond wrote:. . . the validation has to occur before the copying or else it creates an error. . . .
Are you sure nobody will try to modify the List in your objects? Can you trust yourself not to write add or remove calls on that List? Either what you had before, or what I wrote above. If you are keeping a tight rein on that class and not modifying the List, then does it make any difference?
. . . What would be the best option for receiving a list in the constructor to ensure that it can't be modified after object creation?
Yes. But if you are never modifying the List inside your class, any of 2 3 4 will do nicely. Remember that a plain simple copy will allow users to modify the List after receipt, but that is not your problem. Your class will be correct with any of 2 3 or 4.
3. Would option 4 be okay in the getter for the list, example:. . .
This is where we see there are several ways to skin a cat.
Junilu Lacar wrote:. . . What would make more sense is if it returned a boolean true if the list checks out, false otherwise. In that case, the method would probably be better named something like isValid(). . . .