Is it better to return an empty task or null? c#

Question

I have an asynchronous method which will look for a jobId for a job scheduling service through an Api.

if it finds no results is it better to return an empty task or null?

As i understand when returning a collection it is better to return an empty collection rather than null and using objects its better to return null than an empty object; but with tasks i am unsure which is best. See attached method.

Thank you

   public virtual Task<int> GetJobRunIdAsync(int jobId)
        {
            var jobMonRequest = new jobmonRequest(true, true, true, true, true, 
            true, true, true, true, true, true, true,
            true,
            true, true, true, DateTime.Today, jobId, null, 0, null, null,
            null, null, 0, 0);

        var jobMonResponseTask = Client.jobmonAsync(jobMonRequest);

        var jobTask = jobMonResponseTask.ContinueWith(task =>
        {
            if (jobMonResponseTask.Result == null )
            {
                var empty = new Task<int>(() => 0); // as i understand creating a task with a predefined result will reduce overhead.

                return empty.Result;   // || is it better to just return null?
            }
            if (jobMonResponseTask.Result.jobrun.Length > 1)
            {
                throw  new Exception("More than one job found, Wizards are abound.");
            }
              return jobMonResponseTask.Result.jobrun.Single().id;
        });

        return jobTask;
    }

Answer 1

if it finds no results is it better to return an empty task or null?

There's a couple things to consider here:

First, you should never return a null Task. In the async world, a null task just doesn't make sense. Task represents the execution of the asynchronous method, so for an asynchronous method to return a null task is like telling the calling code "you didn't really just call this method" when of course it did.

So, a Task/Task<T> returned from a method should never, ever be null. However, you still have the option of returning a null value inside a regular task. That is up to you.

with tasks i am unsure which is best.

The task is just a wrapper. The underlying logic is still the same. Think of how this method would look if it were synchronous; would your return type be int and return 0 if nothing was found, or would your return type be int? and return null if nothing was found? After making that choice for a synchronous method, then wrap it in Task<T> for the asynchronous method.

As a final note, I must say:

• Do not ever, ever use the Task constructor.
• Avoid Task<T>.Result; use await instead.
• Do not use ContinueWith; use await instead.

Your method can be drastically simplified:

public virtual async Task<int> GetJobRunIdAsync(int jobId)
{
  var jobMonRequest = ...;
  var jobMonResponse = await Client.jobmonAsync(jobMonRequest);
  if (jobMonResponse == null)
    return 0;
  if (jobMonResponse.jobrun.Length > 1)
    throw  new Exception("More than one job found, Wizards are abound.");
  return jobMonResponse.jobrun.Single().id;
}

Or, if you want to return a value (not task) of null:

public virtual async Task<int?> GetJobRunIdAsync(int jobId)
{
  var jobMonRequest = ...;
  var jobMonResponse = await Client.jobmonAsync(jobMonRequest);
  if (jobMonResponse == null)
    return null;
  if (jobMonResponse.jobrun.Length > 1)
    throw  new Exception("More than one job found, Wizards are abound.");
  return jobMonResponse.jobrun.Single().id;
}

Answer 2

If you really want to return null from async method, you can use Task.FromResult(null)

For example:

public async Task<FileInfo> GetInfo()
{
    return await Task.FromResult<FileInfo>(null);
}

Answer 3

The answer from Stephen Cleary explains it perfectly: do never return null, or you'll provoke null reference exceptions but I want to add something:

• if your function returns a Task, return a completed task, which can be done by returning Task.CompletedTask
• if your function returns a Task<T> return a completed task of T, which can be done with Task.FromResult<TResult>(TResult)

If you return null instead of a completed task, this code will throw a null reference exception:

await FunctionThatShouldRetunrTaskButReturnsNull();

and it's a bit difficult to understand what's going on even when you see it in the debugger.

So, never, never, return a null from a non-async function that returns a Task.

Explanation:

• in a non-async function, that returns a Task or Task<T>, you need to create the task explicitly, and there is the danger of returning null instead of a task.
• in an async function that returns a Task or Task<T>, you simply return or return a value, and the result of the function is implicitly converted to a task, so there is no danger of returning null.

Answer 4

My personal preference is to avoid null whenever possible. This forces the caller to implement checking for the return value and reduces unintentional NullReferenceException.

The only time I would use null is for a value-type return. Nullable value types provide the HasValue and Value properties so the caller can do:

var jobId = api.GetJobRunIdAsync(1234).Result; //note: highly recommend using async/await here instead of just returning a task
if(jobId.HasValue)
{
   var result = DoSomethingWithId(jobId);
   //continue processing...
}

I think this would work well in the example you have provided, since you are returning int.

When returning a collection, I would prefer an empty collection instead of a null object. This requires less branching, which makes the code easier to read and test - if a null collection is returned, you end up with something like:

var results = await GetResultsForJobId(1234);
if(results != null) {}
// or results?.SomeLinqOperation();

With an empty collection this is simply

var results = await GetResultsForJobId(1234);
results.SomeLinqOperation();

For other non-collection reference types, I would suggest implementing a Maybe<T> or Optional<T>, which can be used with reference types in a similar way to Nullable<T>. An example of one such implementation can be found on GitHub at https://github.com/nlkl/Optional. A simpler version could be:

public struct Optional<T>
{
    private static readonly Optional<T> _readOnlyEmpty = new Optional<T>();
    public static Optional<T> Empty => _readOnlyEmpty;

    public T Value { get; }

    public bool HasValue { get; private set; }

    public Optional(T value)
        : this()
    {
        Value = value;
        HasValue = true;
    }

    public static implicit operator Optional<T>(T value)
    {
        return new Optional<T>(value);
    }

    public static implicit operator T(Optional<T> optional)
    {
        return optional.Value;
    }
}

Answer 5

There are two points here. First the return value where there is no valid result. I would change the return type to int? and return null for the jobRun Id to indicate to the caller that there is no valid value.

The other part of how to return a Task is detailed here If my interface must return Task what is the best way to have a no-operation implementation?

Answer 6

It is better to return an empty collection than null because collections will often implement IEnumerable and as such will be iterated through via foreach(var item in collection).

Foreach will run into a NullReferenceException if the collection object is null instead of an empty collection. Returning an empty collection will just avoid program crashes due to forgetting a null reference check in that case and is simply more intuitive.

Also a collection's objects on occasion can be created only moment they are needed for example via the yield statement. Before that, the result would simply not exist yet and thus null might be problematic here, so returning an empty collection instead of null makes sense here.

If you return a single object however, returning null is perfectly valid if it is possible that the entity the object represents does not exist and might be different from an empty object (which might have properties intialized to default values etc.). If it might fail, you will need to check if it did fail anyways, so null in this case is not a problem and in fact the desired result if there is nothing there that the reference could represent.

As for the Task. The Task itself is necessary to check or wait for its completion. If you need to be sure the Task has completed, then return an empty Task. If you do not need to check for the Task completion and just fire and forget about it, then what is the purpose of retuning anything?