Can I return a null in a function if something goes wrong?

Question

Lets say I have a function that should return me the information about all textfiles in a folder:

public static FileInfo[] GetTxtFiles()
{
  //...
  FileInfo[] files = directory.GetFiles("*.txt");
  return files;
}

Thats fine. Now lets say the directory is empty or there are no .txt files or something like that, then I get an exception. I thought I could return a null if something goes wrong, something like this:

public static FileInfo[] GetTxtFiles()
{
  try { //...
    FileInfo[] files = directory.GetFiles("*.txt");
    return files;
  } 
  catch (Exception e) 
  {
    logger.Error(e);
    return null;
  } 
}

Now before I call this function I can always check

if (GetTxtFiles() != null) {
  //do something
}

Is this alright? Or is it bad coding if Im returning null? Any suggestions? Thank you

Answer 1

It is "preferred" to return an empty array and check its count instead of null checks.
Check this StackOverflow discussion where this has been ripped to pieces.

A relevant excerpt from the above link:

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.

Answer 2

I would probably not return null.

If there was an error in processing, throw an exception so that it can be traced.

If there are no files to check but the directory exists (or MAYBE if it doesn't) I'd return an empty collection.

If the directory doesn't exist, I'd make it a pre-condition (it would throw an exception), however as a pre-condition caller will be able to test the condition before calling, the exception therefore becomes more like documentation, reminder and bug-checking facility but you'd expect it to never happen in the field and you wouldn't write a try/catch for it because you know it couldn't happen (you create the directory before hand).

Answer 3

In your case, if you don't have any files that you need to return you should return an empty collection, rather than null, as has been beaten to death by many other answers.

However, that is entirely different from error cases that result in exceptions. If everything worked out just fine and you just have nothing to return you should return an empty collection, but if there was an exception you shouldn't always catch it, suppress it, and pretend that everything is fine. If the folder does exist (rather than just being empty), if the user doesn't have access to view the files, if the hard drive died mid-run, or any other entirely unexpected exception this method clearly isn't capable of dealing with it. The appropriate solution is to let it bubble up to until it reaches someplace that can appropriate handle it.

Answer 4

Your solution is fine except that I wouldn't check the result like this

if (GetTxtFiles() != null) {
  //do something
}

because if it is not null you'll have to call the method twice. I would rather store the result and then check it for null value.

Answer 5

If your method was in an API, an null value would give the caller no information about what happened. Returning an empty collection is slightly better but not great because it could be interpreted as "the method was successful but it found no files".

In your situation, I would catch a FileNotFoundException in the calling code. Only return an empty array if you truly found nothing.

Answer 6

In general its better not to return a null but rather an empty array as shown below. In addition to the null values, it's not a good practice to depend on exceptions for logic flow and you should strive to check for conditions that might cause exceptions. Exception handing should be handled at a higher level than in this method.

I am pretty sure that if the directory is empty or there are no .txt files or something like that, then you will not get an exception. As per MSDN the GetFiles method can throw a DirectoryNotFoundException meaning that the path is invalid, such as being on an unmapped drive. Based on the above its advisable to make sure the path exists before calling the GetFiles since exception handling is expensive. Based on this I would change the code to

public static FileInfo[] GetTxtFiles()
{
    return Directory.Exists(path) ? directory.GetFiles("*.txt") : new FileInfo[] {};
} 

As you can see I do not have any exception handling at all since it is not needed anymore.

Answer 7

It is fine.

However, Joshua Bloch recommends to return an empty array rather than null in Effective Java for good reason (see Item 43).

Answer 8

In addition to the other answers, if you're concerned this will happen often and cause a large number of object allocations (via a new FileInfo[0] in the catch block), feel free to have an already-created empty array handy:

private static readonly FileInfo[] emptyFileInfo = new FileInfo[0];

public static FileInfo[] GetTxtFiles()
{
  try { //...
    FileInfo[] files = directory.GetFiles("*.txt");
    return files;
  } 
  catch (Exception e) 
  {
    logger.Error(e);
    return emptyFileInfo;
  } 
}

Answer 9

Cause you return FileInfo[] it is good to catch exception that thrown when no files in directory and return new FileInfo[0]

Answer 10

Instead of going for an array you can even pick List as a return. Something like this.

public static List<FileInfo> GetTxtFiles()
{
 try
  {
 //...
 List<FileInfo> fileInfo = dirInfo.EnumerateFiles("*.txt").ToList();
 return fileInfo;
  }
 catch(Exception e)
  {
     logger.Error(e);
  }
}

This would eliminate the need to see if this function returns a null, instead while making a call you can see the Count property value.

if (GetTxtFiles().Count > 0) 
{
 //do something
}

Answer 11

Having an empty result set is not an 'exception'. If you cannot access the directory or the 'directory' variable itself is null it is an exception. Here is what Msdn (http://msdn.microsoft.com/en-us/library/8he88b63.aspx ) says about DirectoryInfo.GetFiles(string) method -- "If there are no files in the DirectoryInfo, this method returns an empty array."

Answer 12

Yes, that is perfectly fine. Just about every library/framework returns null. It is the callers responsibility to check the return value.

You can also return a number to indicate the type of error to the caller of the function, or you can throw the exception up to the calling function as well.

Answer 13

yes that's right your solution