That's kind of a general question (but I'm using C#), what's the best way (best practice), do you return null or empty collection for a method that has a collection as a return type ?
This question is related to
c#
collections
One could argue that the reasoning behind Null Object Pattern is similar to one in favour of returning the empty collection.
From the perspective of managing complexity, a primary software engineering objective, we want to avoid propagating unnecessary cyclomatic complexity to the clients of an API. Returning a null to the client is like returning them the cyclomatic complexity cost of another code branch.
(This corresponds to a unit testing burden. You would need to write a test for the null return case, in addition to the empty collection return case.)
It seems to me that you should return the value that is semantically correct in context, whatever that may be. A rule that says "always return an empty collection" seems a little simplistic to me.
Suppose in, say, a system for a hospital, we have a function that is supposed to return a list of all previous hospitalizations for the past 5 years. If the customer has not been in the hospital, it makes good sense to return an empty list. But what if the customer left that part of the admittance form blank? We need a different value to distinguish "empty list" from "no answer" or "don't know". We could throw an exception, but it's not necessarily an error condition, and it doesn't necessarily drive us out of the normal program flow.
I've often been frustrated by systems that cannot distinguish between zero and no answer. I've had a number of times where a system has asked me to enter some number, I enter zero, and I get an error message telling me that I must enter a value in this field. I just did: I entered zero! But it won't accept zero because it can't distinguish it from no answer.
Reply to Saunders:
Yes, I'm assuming that there's a difference between "Person didn't answer the question" and "The answer was zero." That was the point of the last paragraph of my answer. Many programs are unable to distinguish "don't know" from blank or zero, which seems to me a potentially serious flaw. For example, I was shopping for a house a year or so ago. I went to a real estate web site and there were many houses listed with an asking price of $0. Sounded pretty good to me: They're giving these houses away for free! But I'm sure the sad reality was that they just hadn't entered the price. In that case you may say, "Well, OBVIOUSLY zero means they didn't enter the price -- nobody's going to give a house away for free." But the site also listed the average asking and selling prices of houses in various towns. I can't help but wonder if the average didn't include the zeros, thus giving an incorrectly low average for some places. i.e. what is the average of $100,000; $120,000; and "don't know"? Technically the answer is "don't know". What we probably really want to see is $110,000. But what we'll probably get is $73,333, which would be completely wrong. Also, what if we had this problem on a site where users can order on-line? (Unlikely for real estate, but I'm sure you've seen it done for many other products.) Would we really want "price not specified yet" to be interpreted as "free"?
RE having two separate functions, an "is there any?" and an "if so, what is it?" Yes, you certainly could do that, but why would you want to? Now the calling program has to make two calls instead of one. What happens if a programmer fails to call the "any?" and goes straight to the "what is it?" ? Will the program return a mis-leading zero? Throw an exception? Return an undefined value? It creates more code, more work, and more potential errors.
The only benefit I see is that it enables you to comply with an arbitrary rule. Is there any advantage to this rule that makes it worth the trouble of obeying it? If not, why bother?
Reply to Jammycakes:
Consider what the actual code would look like. I know the question said C# but excuse me if I write Java. My C# isn't very sharp and the principle is the same.
With a null return:
HospList list=patient.getHospitalizationList(patientId);
if (list==null)
{
// ... handle missing list ...
}
else
{
for (HospEntry entry : list)
// ... do whatever ...
}
With a separate function:
if (patient.hasHospitalizationList(patientId))
{
// ... handle missing list ...
}
else
{
HospList=patient.getHospitalizationList(patientId))
for (HospEntry entry : list)
// ... do whatever ...
}
It's actually a line or two less code with the null return, so it's not more burden on the caller, it's less.
I don't see how it creates a DRY issue. It's not like we have to execute the call twice. If we always wanted to do the same thing when the list does not exist, maybe we could push handling down to the get-list function rather than having the caller do it, and so putting the code in the caller would be a DRY violation. But we almost surely don't want to always do the same thing. In functions where we must have the list to process, a missing list is an error that might well halt processing. But on an edit screen, we surely don't want to halt processing if they haven't entered data yet: we want to let them enter data. So handling "no list" must be done at the caller level one way or another. And whether we do that with a null return or a separate function makes no difference to the bigger principle.
Sure, if the caller doesn't check for null, the program could fail with a null-pointer exception. But if there's a separate "got any" function and the caller doesn't call that function but blindly calls the "get list" function, then what happens? If it throws an exception or otherwise fails, well, that's pretty much the same as what would happen if it returned null and didn't check for it. If it returns an empty list, that's just wrong. You're failing to distinguish between "I have a list with zero elements" and "I don't have a list". It's like returning zero for the price when the user didn't enter any price: it's just wrong.
I don't see how attaching an additional attribute to the collection helps. The caller still has to check it. How is that better than checking for null? Again, the absolute worst thing that could happen is for the programmer to forget to check it, and give incorrect results.
A function that returns null is not a surprise if the programmer is familiar with the concept of null meaning "don't have a value", which I think any competent programmer should have heard of, whether he thinks it's a good idea or not. I think having a separate function is more of a "surprise" problem. If a programmer is unfamiliar with the API, when he runs a test with no data he'll quickly discover that sometimes he gets back a null. But how would he discover the existence of another function unless it occurred to him that there might be such a function and he checks the documentation, and the documentation is complete and comprehensible? I would much rather have one function that always gives me a meaningful response, rather than two functions that I have to know and remember to call both.
Empty is much more consumer friendly.
There is a clear method of making up an empty enumerable:
Enumerable.Empty<Element>()
Returning an empty collection is better in most cases.
The reason for that is convenience of implementation of the caller, consistent contract, and easier implementation.
If a method returns null to indicate empty result, the caller must implement a null checking adapter in addition to enumeration. This code is then duplicated in various callers, so why not to put this adapter inside the method so it could be reused.
A valid usage of null for IEnumerable might be an indication of absent result, or an operation failure, but in this case other techniques should be considered, such as throwing an exception.
using System;
using System.Collections.Generic;
using System.Linq;
using NUnit.Framework;
namespace StackOverflow.EmptyCollectionUsageTests.Tests
{
/// <summary>
/// Demonstrates different approaches for empty collection results.
/// </summary>
class Container
{
/// <summary>
/// Elements list.
/// Not initialized to an empty collection here for the purpose of demonstration of usage along with <see cref="Populate"/> method.
/// </summary>
private List<Element> elements;
/// <summary>
/// Gets elements if any
/// </summary>
/// <returns>Returns elements or empty collection.</returns>
public IEnumerable<Element> GetElements()
{
return elements ?? Enumerable.Empty<Element>();
}
/// <summary>
/// Initializes the container with some results, if any.
/// </summary>
public void Populate()
{
elements = new List<Element>();
}
/// <summary>
/// Gets elements. Throws <see cref="InvalidOperationException"/> if not populated.
/// </summary>
/// <returns>Returns <see cref="IEnumerable{T}"/> of <see cref="Element"/>.</returns>
public IEnumerable<Element> GetElementsStrict()
{
if (elements == null)
{
throw new InvalidOperationException("You must call Populate before calling this method.");
}
return elements;
}
/// <summary>
/// Gets elements, empty collection or nothing.
/// </summary>
/// <returns>Returns <see cref="IEnumerable{T}"/> of <see cref="Element"/>, with zero or more elements, or null in some cases.</returns>
public IEnumerable<Element> GetElementsInconvenientCareless()
{
return elements;
}
/// <summary>
/// Gets elements or nothing.
/// </summary>
/// <returns>Returns <see cref="IEnumerable{T}"/> of <see cref="Element"/>, with elements, or null in case of empty collection.</returns>
/// <remarks>We are lucky that elements is a List, otherwise enumeration would be needed.</remarks>
public IEnumerable<Element> GetElementsInconvenientCarefull()
{
if (elements == null || elements.Count == 0)
{
return null;
}
return elements;
}
}
class Element
{
}
/// <summary>
/// http://stackoverflow.com/questions/1969993/is-it-better-to-return-null-or-empty-collection/
/// </summary>
class EmptyCollectionTests
{
private Container container;
[SetUp]
public void SetUp()
{
container = new Container();
}
/// <summary>
/// Forgiving contract - caller does not have to implement null check in addition to enumeration.
/// </summary>
[Test]
public void UseGetElements()
{
Assert.AreEqual(0, container.GetElements().Count());
}
/// <summary>
/// Forget to <see cref="Container.Populate"/> and use strict method.
/// </summary>
[Test]
[ExpectedException(typeof(InvalidOperationException))]
public void WrongUseOfStrictContract()
{
container.GetElementsStrict().Count();
}
/// <summary>
/// Call <see cref="Container.Populate"/> and use strict method.
/// </summary>
[Test]
public void CorrectUsaOfStrictContract()
{
container.Populate();
Assert.AreEqual(0, container.GetElementsStrict().Count());
}
/// <summary>
/// Inconvenient contract - needs a local variable.
/// </summary>
[Test]
public void CarefulUseOfCarelessMethod()
{
var elements = container.GetElementsInconvenientCareless();
Assert.AreEqual(0, elements == null ? 0 : elements.Count());
}
/// <summary>
/// Inconvenient contract - duplicate call in order to use in context of an single expression.
/// </summary>
[Test]
public void LameCarefulUseOfCarelessMethod()
{
Assert.AreEqual(0, container.GetElementsInconvenientCareless() == null ? 0 : container.GetElementsInconvenientCareless().Count());
}
[Test]
public void LuckyCarelessUseOfCarelessMethod()
{
// INIT
var praySomeoneCalledPopulateBefore = (Action)(()=>container.Populate());
praySomeoneCalledPopulateBefore();
// ACT //ASSERT
Assert.AreEqual(0, container.GetElementsInconvenientCareless().Count());
}
/// <summary>
/// Excercise <see cref="ArgumentNullException"/> because of null passed to <see cref="Enumerable.Count{TSource}(System.Collections.Generic.IEnumerable{TSource})"/>
/// </summary>
[Test]
[ExpectedException(typeof(ArgumentNullException))]
public void UnfortunateCarelessUseOfCarelessMethod()
{
Assert.AreEqual(0, container.GetElementsInconvenientCareless().Count());
}
/// <summary>
/// Demonstrates the client code flow relying on returning null for empty collection.
/// Exception is due to <see cref="Enumerable.First{TSource}(System.Collections.Generic.IEnumerable{TSource})"/> on an empty collection.
/// </summary>
[Test]
[ExpectedException(typeof(InvalidOperationException))]
public void UnfortunateEducatedUseOfCarelessMethod()
{
container.Populate();
var elements = container.GetElementsInconvenientCareless();
if (elements == null)
{
Assert.Inconclusive();
}
Assert.IsNotNull(elements.First());
}
/// <summary>
/// Demonstrates the client code is bloated a bit, to compensate for implementation 'cleverness'.
/// We can throw away the nullness result, because we don't know if the operation succeeded or not anyway.
/// We are unfortunate to create a new instance of an empty collection.
/// We might have already had one inside the implementation,
/// but it have been discarded then in an effort to return null for empty collection.
/// </summary>
[Test]
public void EducatedUseOfCarefullMethod()
{
Assert.AreEqual(0, (container.GetElementsInconvenientCarefull() ?? Enumerable.Empty<Element>()).Count());
}
}
}
Think always in favor of your clients (which are using your api):
Returning 'null' very often makes problems with clients not handling null checks correctly, which causes a NullPointerException during runtime. I have seen cases where such a missing null-check forced a priority production issue (a client used foreach(...) on a null value). During testing the problem did not occur, because the data operated on was slightly different.
Empty Collection. If you're using C#, the assumption is that maximizing system resources is not essential. While less efficient, returning Empty Collection is much more convenient for the programmers involved (for the reason Will outlined above).
I call it my billion-dollar mistake…At that time, I was designing the first comprehensive type system for references in an object-oriented language. My goal was to ensure that all use of references should be absolutely safe, with checking performed automatically by the compiler. But I couldn’t resist the temptation to put in a null reference, simply because it was so easy to implement. This has led to innumerable errors, vulnerabilities, and system crashes, which have probably caused a billion dollars of pain and damage in the last forty years. – Tony Hoare, inventor of ALGOL W.
See here for an elaborate shit storm about null in general. I do not agree with the statement that undefined is another null, but it is still worth reading. And it explains, why you should avoid null at all and not just in the case you have asked. The essence is, that null is in any language a special case. You have to think about null as an exception. undefined is different in that way, that code dealing with undefined behavior is in most cases just a bug. C and most other languages have also undefined behavior but most of them have no identifier for that in the language.
Depends on the situation. If it is a special case, then return null. If the function just happens to return an empty collection, then obviously returning that is ok. However, returning an empty collection as a special case because of invalid parameters or other reasons is NOT a good idea, because it is masking a special case condition.
Actually, in this case I usually prefer to throw an exception to make sure it is REALLY not ignored :)
Saying that it makes the code more robust (by returning an empty collection) as they do not have to handle the null condition is bad, as it is simply masking a problem that should be handled by the calling code.
Depends on your contract and your concrete case. Generally it's best to return empty collections, but sometimes (rarely):
null might mean something more specific;null.Some concrete examples:
null would mean the element is missing, while an empty collection would render a redundant (and possibly incorrect) <collection />From the Framework Design Guidelines 2nd Edition (pg. 256):
DO NOT return null values from collection properties or from methods returning collections. Return an empty collection or an empty array instead.
Here's another interesting article on the benefits of not returning nulls (I was trying to find something on Brad Abram's blog, and he linked to the article).
Edit- as Eric Lippert has now commented to the original question, I'd also like to link to his excellent article.
I would argue that null isn't the same thing as an empty collection and you should choose which one best represents what you're returning. In most cases null is nothing (except in SQL). An empty collection is something, albeit an empty something.
If you have have to choose one or the other, I would say that you should tend towards an empty collection rather than null. But there are times when an empty collection isn't the same thing as a null value.
I like to give explain here, with suitable example.
Consider a case here..
int totalValue = MySession.ListCustomerAccounts()
.FindAll(ac => ac.AccountHead.AccountHeadID
== accountHead.AccountHeadID)
.Sum(account => account.AccountValue);
Here Consider the functions I am using ..
1. ListCustomerAccounts() // User Defined
2. FindAll() // Pre-defined Library Function
I can easily use ListCustomerAccount and FindAll instead of.,
int totalValue = 0;
List<CustomerAccounts> custAccounts = ListCustomerAccounts();
if(custAccounts !=null ){
List<CustomerAccounts> custAccountsFiltered =
custAccounts.FindAll(ac => ac.AccountHead.AccountHeadID
== accountHead.AccountHeadID );
if(custAccountsFiltered != null)
totalValue = custAccountsFiltered.Sum(account =>
account.AccountValue).ToString();
}
NOTE : Since AccountValue is not null, the Sum() function will not
return null., Hence I can use it directly.
If an empty collection makes sense semantically, that's what I prefer to return. Returning an empty collection for GetMessagesInMyInbox() communicates "you really do not have any messages in your inbox", whereas returning null might be useful to communicate that insufficient data is available to say what the list that might be returned ought to look like.
Returning null could be more efficient, as no new object is created. However, it would also often require a null check (or exception handling.)
Semantically, null and an empty list do not mean the same thing. The differences are subtle and one choice may be better than the other in specific instances.
Regardless of your choice, document it to avoid confusion.
We had this discussion among the development team at work a week or so ago, and we almost unanimously went for empty collection. One person wanted to return null for the same reason Mike specified above.
There is one other point that hasn't yet been mentioned. Consider the following code:
public static IEnumerable<string> GetFavoriteEmoSongs()
{
yield break;
}
The C# Language will return an empty enumerator when calling this method. Therefore, to be consistant with the language design (and, thus, programmer expectations) an empty collection should be returned.
Source: Stackoverflow.com