Be conservative in what you accept and conservative in what you send
It's tempting to write overly-friendly APIs: public void setDeliveries(List<Delivery> deliveries) {
if (deliveries == null) {
deliveries = Collections.emptyList();
}
this.deliveries = Collections.unmodifiableList(
new ArrayList<Delivery>(deliveries));
}Please don't do this:
It masks bugs. The caller might be passing the wrong deliveries object, such as
orderDeliveries when they intended this.orderDeliveries.It makes your code difficult to maintain. Assume that this method has several callers and you'd like to override it. Now the subclass needs to maintain this exact same behaviour. You'll face similar issues if you introduce an interface or do other refactorings.
Instead, constrain your inputs as tightly as possible:
public void setDeliveries(List<Delivery> deliveries) {
if (deliveries == null) {
throw new NullPointerException("deliveries");
}
this.deliveries = Collections.unmodifiableList(
new ArrayList<Delivery>(deliveries));
}Note that we would have the same behaviour without the explicit null check because
new ArrayList<deliveries> throws as well. But I'm coding as much for the reader as for the compiler so I like to be as intentional as possible.Finally, consider writing or adopting a utility class to encourage writing concise preconditions.
