3

i don't want to pass a null value to the view tier, so if i do something like:

  public List<Object> getListObjFoo(){
    List<Object> listObj = datasource.getAll();
    return listObj != null ? listObj : new ArrayList<Object>();
  }

it should be considered a good or a bad practice? and the use of "Optional" in "Guava" from google is better than doing this? why?

cнŝdk
  • 31,391
  • 7
  • 56
  • 78
tiagomac
  • 461
  • 1
  • 7
  • 18
  • A null check is *always* better than an object that you don't need. BTW you will still encounter NPEs when you try calling methods on the Objects (that should be) present inside the arraylist. – TheLostMind Sep 23 '14 at 13:24
  • Is there a specific reason you don't want to pass `null`? A null check in your view should be a special case, and a completely different meaning from if your dataSource were to possibly return an empty list. – Compass Sep 23 '14 at 13:26
  • 3
    `List` is *generally* a bad practice. Now a specific `List` type is okay I guess. Finally, depending on context, this seems fine to me. – Elliott Frisch Sep 23 '14 at 13:26
  • 3
    @TheLostMind: _A null check is always better than an object that you don't need._ I strongly disagree with this statement. There is definitely a place for both variants. – Keppil Sep 23 '14 at 13:29
  • 4
    You should probably return `Collections.emptyList()` instead of `new ArrayList<>()` – assylias Sep 23 '14 at 13:30
  • Leave null if you don't this could cause logical errors down the road. – brso05 Sep 23 '14 at 13:30
  • @Keppil - what if his *very next* line of code is `list.get(0);`?. Somewhere he will have to make `isEmpty() / null` check. – TheLostMind Sep 23 '14 at 13:33
  • 1
    Two questions: could `dataSource.getAll()` itself return an empty `ArrayList`, and if so, is it important to distinguish between that and `null`? – chepner Sep 23 '14 at 13:33
  • 1
    @TheLostMind: What if all the calling method does is a `for Object o: list`? Then there is no need for an explicit `isEmpty()` check if this method never returns `null`. – Keppil Sep 23 '14 at 13:34
  • @Keppil - I agree. But you are banking on the OP to use `for Object o: list`. Next, yes, an empty list should work (and have less impact on memory because of its *Singleton* nature). I would rather use an *Empty list* instead of `new ArrayList()` – TheLostMind Sep 23 '14 at 13:41

3 Answers3

5

Not checking for null makes your code much cleaner, but it has its cost (it depends on the complexity of object you want to create).

Returning Collection.emptyList() instead of null is a good practise as its cost is equal to zero.

update

Existing example of returning empty list can be even seen at Java Persistence API in method Query#getResultList(). It always returns empty list of results if no query results have been found.

Maciej Dobrowolski
  • 11,561
  • 5
  • 45
  • 67
5

Your code is bad practice because it allocates a new instance. Try to use Collection.emptyList() instead.

Drawback: If the consumer of your code wants to change the list, they need to make a copy of the list. But that is good practice since many frameworks return immutable lists and as a consumer, you can never be sure unless the API actually says what you are allowed to do with the result.

Returning null is bad because that exports an implementation detail of your code to the consumer. Or to put it another way around: If you change your implementation and you suddenly find you want to return null, you probably need to add checks in all the places where the API is used. So it hampers your ability to evolve your code and your API.

For other (non-collection) types, you should look at the "optional" pattern as described here: https://stackoverflow.com/a/16218718/34088

This pattern allows you to tell consumers of your API that some method might return "nothing" without ever returning null. That way, consumers know what to expect.

Community
  • 1
  • 1
Aaron Digulla
  • 321,842
  • 108
  • 597
  • 820
4

Null is a legacy from the languages with direct memory management, where pointer could point into nowhere. Modern languages and practices encourage to avoid using nulls. As a rule of thumb, try to use non-nullable variables where possible and consider using Optional instead of using nullables. You can read more about reasons here.

Now regarding your case. I assume the question was "what I should return, an empty collection, null or empty Optional?". The short answer is: it depends on your use case. Using nullable result or Optional clearly tells, that the whole result is optional. For example, for web view it can tell that you may or may not render the whole block with list. On the other hand, returning non-nullable collection that may be empty means that you should always render the corresponding block, but in some cases it can have no elements in it.

Aivean
  • 10,692
  • 25
  • 39