Return empty collections in place of nulls

[TBuc]

When calling a getXyz() method on a Mail object, where xyz is some object implementing Collection, if xyz is null then an empty collection will be returned (currently returns null).

[SendGridDX]

All committers have signed the CLA.

[thinkingserious]

Hi @TBuc!

Thanks for the PR!

Could you please provide some explanation why we need to return empty collections rather than nulls?

With Best Regards,

Elmer

[TBuc]

Hi @thinkingserious,

I kept it short at first, but I can do it for sure!

In general, nowadays avoiding returning nulls is generally a best practice, especially for collections. When fetching a collection, I expect to get a collection. If it has no objects in it, it will just be an empty collection. A potential NullPointerException should be avoided if not specifically needed.

There are many alternatives to achieve this goal, i.e.:

1. • Eagerly initializing collections
2. • Returning an empty collection when null (return new SomeCollection<T>();)
3. • Initializing a collection when null (would need synchronization: I know now the addition methods aren't synchronized, but I would not make it worse)
4. • Returning an Optional<T extends Collection<T>> object in place of the current <T extends Collection<T>>

In my PR I've opted for the solution n.2 since it has the lowest impact on the existing code.

Best regards,
Antonio

[vaskoz]

@TBuc You're right about returning empty collections instead of nulls (Effective Java 2nd edition Item 43). Also, quoting directly from Item 43, "To eliminate the overhead from creating an empty collection or array, return the immutable empty collections in java.util.Collections".

Would you mind using the java.util.Collections immutable empty collections instead?

return Collections.emptyList();
and 
return Collections.emptyMap();

instead of:

return new ArrayList<Email>()
and 
return new HashMap<String, String>();

[TBuc]

@vaskoz that's great! I've read Bloch quite a few years ago, and I did really not remember such immutable collections do exist. I will modify my code accordingly to your great suggestion. Thank you for pointing out!

I would only add the type parameterization to the code you propose, in order to return the exact type for the collection objects in order to maintain the current return types. So that, i.e., what you proposed:
public List<Email> getBccs() { [...] return Collections.emptyList(); [...] }
would become:
public List<Email> getBccs() { [...] return Collections.<Email>emptyList(); [...] }
Please let me know whether you agree or have a different opinion on it. I will wait for your feedback before enhancing the PR.

[TBuc]

Since one day has passed, I suppose it's OK and I've updated the PR.
@vaskoz please feel free to comment if you have any further suggestion to share.

[vaskoz]

@TBuc Thanks so much! I have no comments.

[thinkingserious]

Hello @TBuc,

Thanks again for the PR!

We want to show our appreciation by sending you some swag. Could you please fill out this form so we can send it to you? Thanks!

Team SendGrid DX