208

What is the best practice when returning data from functions. Is it better to return a Null or an empty object? And why should one do one over the other?

Consider this:

public UserEntity GetUserById(Guid userId)
{
     //Imagine some code here to access database.....

     //Check if data was returned and return a null if none found
     if (!DataExists)
        return null; 
        //Should I be doing this here instead? 
        //return new UserEntity();  
     else
        return existingUserEntity;
}

Lets pretend that there would be valid cases in this program that there would be no user information in the database with that GUID. I Would imagine that it would not be appropriate to throw an exception in this case?? Also I am under the impression that exception handling can hurt performance.

7wp
  • 12,505
  • 20
  • 77
  • 103
  • 4
    I think you mean `if (!DataExists)`. – Sarah Vessels Oct 26 '09 at 19:20
  • 107
    This is an architectural question and is perfectly appropriate. The OP's question is valid regardless of the business problem it is trying to solve. – Joseph Ferris Oct 26 '09 at 19:21
  • 2
    This question has already been sufficiently answered. I think this is a very interesting question. – John Scipione Oct 27 '09 at 01:23
  • 'getUser()' should return null. 'getCurrentUserInfo()' or 'getCurrentPermissions()', OTOH, would be more revealing questions -- they should return a _non-null answer object_ regardless of who/ or whether anyone is logged in. – Thomas W May 03 '13 at 02:46
  • Possible duplicate of [Is it better to return null or empty collection?](https://stackoverflow.com/questions/1969993/is-it-better-to-return-null-or-empty-collection) – Bergi Jun 08 '17 at 02:43
  • 2
    No @Bergi the other one is a duplicate. Mine was asked first, in October, the other was asked 3 moths later in December. Plus the other one talks about a collection which is slightly different. – 7wp Jun 16 '17 at 14:50
  • @7wp Yeah, that's why I only commented instead of voting. However, I find the notion of "empty" to be slightly confusing, how can an object be empty if it's not a collection of some sort? – Bergi Jun 16 '17 at 16:34
  • @Bergi, an empty object is any object whose data members are initialized to default values (which are not meaningful in most cases) – user1451111 Nov 05 '17 at 10:12

31 Answers31

206

Returning null is usually the best idea if you intend to indicate that no data is available.

An empty object implies data has been returned, whereas returning null clearly indicates that nothing has been returned.

Additionally, returning a null will result in a null exception if you attempt to access members in the object, which can be useful for highlighting buggy code - attempting to access a member of nothing makes no sense. Accessing members of an empty object will not fail meaning bugs can go undiscovered.

ljs
  • 37,275
  • 36
  • 106
  • 124
  • 21
    You should be throwing an exception, not swallowing the problem and returning null. At a minimum, you should log this and then continue. – Chris Ballance Oct 26 '09 at 18:50
  • 130
    @Chris: I disagree. If the code clearly documents that the return value is null, it's perfectly acceptable to return null if no result is found to match your criteria. Throwing an exception should be your LAST choice, not your FIRST one. – Mike Hofer Oct 26 '09 at 18:53
  • 12
    @Chris: On what basis do you decide this? Adding logging to the equation **certainly** seems excessive. Let the consuming code decide what--if anything--should be done in the case of no user. As with my previous comment, there's absolutely no issue with returning a value that is universally defined as "no data". – Adam Robinson Oct 26 '09 at 18:53
  • @Chris: I don't understand your comment? What should be throwing an exception? The function? Why would it be an error for it to return null? It may be an error that in the context of the call site a null is an unacceptable return value but thats not the function's decision to make. – AnthonyWJones Oct 26 '09 at 18:54
  • It can be acceptable to return null, but relying on returning null and then NullReference throwing higher in the stack is incorrect, because it removes the actual problem one call away from the origin site. – Rex M Oct 26 '09 at 18:54
  • The original problem is "user does not exist". We throw an exception when we demand a file path that doesn't exist. Both patterns are appropriate - it just depends. – Rex M Oct 26 '09 at 18:55
  • 2
    The function itself isn't in error by returning null, rather it's indicating that nothing has been returned. Of course the caller should be testing for null, I agree, however it does add an extra line of defence knowing that even if a null object sneaks through it will raise an exception. That is only icing on the cake compared to the fact you get to indicate that nothing has been returned in a universally understood way. If you simply return an empty object, how would you test that it was empty and throw an exception? It's more difficult and messy to implement that way. – ljs Oct 26 '09 at 18:56
  • @Rex: As I said in response to your own answer, the file system scenarios are all in situations where you're performing an operation on the file, not simply bringing back an object that represents the file. – Adam Robinson Oct 26 '09 at 18:59
  • 17
    I'm a little baffled that a Microsoft developer believes that "returning null" equates to "swallowing the problem." If memory serves, there are tons methods in the Framework where methods null is returned if there's nothing that matches the caller's request. Is that "swallowing the problem?" – Mike Hofer Oct 26 '09 at 19:01
  • 2
    I think both null and exception are valid options. However, if the method throws an exception, you should give the user a way to avoid the exception - for example, by publicly exposing IsValidId(Guid id), or "GetListOfIds()". – Mathias Oct 26 '09 at 19:31
  • 1
    If you postulate that you agree the caller should test for null, why proclude empty objects? Instead of obj != null, you would just be doing obj != default(x). I would think if you are explicitly checking the return, either method would be appropriate. – Joseph Ferris Oct 26 '09 at 19:31
  • 3
    @joseph: You could also implement any of a number of other arbitrary means to return a reserved value, but there is *already* a reserved value for "no data": `null`. – Adam Robinson Oct 26 '09 at 19:36
  • 1
    @Adam - one can also argue that there are two, since default() implies, through this usage, the equivalent of the return saying "I have no apples" or null being the equivalent of just saying "I have none". I think that the point is that I can't find justification for one being better than the other and it really is a matter of personal preference. Return a default is actually quite similar to what SQL Server does when you have a query with no rows. It still returns the table schema, but no records. This isn't about "any number of arbitrary ways", this is specifically about comparing two. – Joseph Ferris Oct 26 '09 at 20:28
  • 5
    Last but not least there would be `bool GetUserById(Guid userId, out UserEntity result)` - which I would prefer to the "null" return-value, and which isn't as extreme as throwing an exception. It allows beautiful `null`-free code like `if(GetUserById(x,u)) { ... }`. – Marcel Jackwerth Oct 27 '09 at 03:44
  • 3
    I guess the choice between returning null or throwing an exception depends on a lot of other factors. It should be consistant with the rest. It should not return null on one error and throw an exception on another error. Exceptions should only be used for signaling errors, not changing the program flow. If (in this example here) !DataAvailable is a standard case that can happen very often, I'd go for returning null. If the application assumes that normally the DB is there, Data is there, I'd go for the exception, because then I can handle the error in any part of the app, not just the caller. – Tobias Langner Oct 28 '09 at 13:15
  • 1
    IMO, empty objects is data and null, doesn't. So if you want to know what happend with this method and this method return null, well this method did nothing. – Agusti-N Oct 28 '09 at 13:48
  • 1
    Thanks everyone for all of your suggestions! I decided to choose this answer as my accepted answer because this is the approach I decided to take. However, I really like all of the other arguments and I will continue to read any new ones suggested. – 7wp Oct 28 '09 at 21:04
  • 2
    @Tobias: "It should not return null on one error and throw an exception on another error." I think it can make sense to return null or throw an exception. If null is documented to mean "no matching user found in the database", then null is an expected return value for this particular function, although it could still be an application-level error. On the other hand, if the database server is down, I wouldn't want the function to return null. I would want to get an exception. – Mike Spross Oct 30 '09 at 03:42
  • 3
    returning NULL is a shortcut to `bool GetUserById(GUID id, out UserEntity user)` which I find perfectly acceptable. Why take 2 steps to avoid an exception.. would you do this? `if(userexists(uid)) { }` and make 2 calls to the database? – Mike Gleason jr Couturier Nov 02 '09 at 01:13
  • Ok, so what methods/properties that return a list, where there's nothing to go in the list. Should they return null, or an empty list? For example xmlNode.SelectNodes(xPath) will always return an empty list rather than null (as an aside, Resharper really irritates me when it recommends I check the result for null). Personally, I find this makes my code a lot cleaner, not having to check for null first. – Flynn1179 Jun 14 '10 at 10:21
  • Exceptions occur when you try to do 'B' on 'A' and there is a hiccup, like if you try to access a function of an object and that object is null. Returning an empty object instead of null is like cutting your code down the spine and throwing vinegar on it. Return null, and check for null. –  Feb 12 '14 at 15:17
  • I am partial to returning an empty collection rather than null at any time. If an Enumerable (e.g. LINQ) extension method is used on a collection and no elements are found, the result is an empty collection. Seems to reason that consistency is better and in line with the framework's design. – IAbstract Mar 12 '14 at 22:01
  • 1
    Returning null is the only reasonable response. Returning an empty object is tantamount to suicide ("Oh, yeah, it returned an object, but it is empty so that means it is an error" "IN WHAT UNIVERSE DO YOU LIVE IN?!?!). Throwing an exception is of course not even vaguely reasonable. Exceptions should be used judiciously. This is not the place for one. – Lloyd Sargent Dec 01 '14 at 16:57
  • Please tell this to the CSLA folks. – Wonko the Sane Jul 10 '15 at 13:15
  • My opinion: in a strongly typed language such as C#, null is NOT a type, and therefore is incapable of carrying any useful information. Therefore, null should never be returned. Instead, return a type that provides useful information (the 'empty' object sounds reasonable). Just my two cents on a very important question. – Rock Anthony Johnson Jun 15 '17 at 17:05
44

It depends on what makes the most sense for your case.

Does it make sense to return null, e.g. "no such user exists"?

Or does it make sense to create a default user? This makes the most sense when you can safely assume that if a user DOESN'T exist, the calling code intends for one to exist when they ask for it.

Or does it make sense to throw an exception (a la "FileNotFound") if the calling code is demanding a user with an invalid ID?

However - from a separation of concerns/SRP standpoint, the first two are more correct. And technically the first is the most correct (but only by a hair) - GetUserById should only be responsible for one thing - getting the user. Handling its own "user does not exist" case by returning something else could be a violation of SRP. Separating into a different check - bool DoesUserExist(id) would be appropriate if you do choose to throw an exception.

Based on extensive comments below: if this is an API-level design question, this method could be analogous to "OpenFile" or "ReadEntireFile". We are "opening" a user from some repository and hydrating the object from the resultant data. An exception could be appropriate in this case. It might not be, but it could be.

All approaches are acceptable - it just depends, based on the larger context of the API/application.

Rex M
  • 142,167
  • 33
  • 283
  • 313
  • Someone voted you down and I voted you back up, since this doesn't seem like a bad answer to me; except: I wouldn't ever throw an exception when finding no user in a method like the one the poster gives. If finding no user implies an invalid ID or some such exception-worthy problem, that should happen higher up -- the throwing method needs to know more about where that ID came from, etc. – Jacob Mattison Oct 26 '09 at 18:50
  • (I'm guessing the downvote was an objection to the idea of throwing an exception in a circumstance like this.) – Jacob Mattison Oct 26 '09 at 18:51
  • 1
    Agreed until your last point. There's no violation of SRP by returning a value that is universally defined as "no data". That's like stating that a SQL database should return an error if a where clause produces no results. While an Exception is a valid design choice (though one that would irritate me as a consumer), it's not any "more correct" than returning null. And no, I'm not the DV. – Adam Robinson Oct 26 '09 at 18:51
  • @JacobM we throw exceptions when we demand a filesystem path that doesn't exist, not return null, but not from databases. So clearly both are appropriate, which is what I'm getting at - it just depends. – Rex M Oct 26 '09 at 18:53
  • @Rex: Look at the `FileNotFoundException` cases. They're all under circumstances where you're performing an *operation* on the file (opening a stream, deleting it, etc.). I would bet good money that if, say, `DirectoryInfo` had a `GetFile` function that returned a single child file, it would return `null` rather than throwing an exception when the file didn't exist. `GetFiles()` returns an empty array if no files are found. I'd be interested to see an example where a function *other than an operation on the file itself* threw `FileNotFound` in the event that it simply didn't exist. – Adam Robinson Oct 26 '09 at 18:56
  • @Adam indeed, I would consider this method to be analogous to "OpenFile" - we are opening the user from the repository. – Rex M Oct 26 '09 at 19:04
  • The `FileInfo` constructor accepts a path. If the path doesn't exist, it does not throw a `FileNotFoundException`. It has a property `Exists` which will tell you whether the file exists or not. – Ryan Alford Oct 26 '09 at 19:16
  • This is a BUSINESS DOMAIN MODEL Question, not a technical question. – Charles Bretana Oct 26 '09 at 19:16
  • @Charles we don't know that - it could just as well be an API design question. Lots of systems have users. – Rex M Oct 26 '09 at 19:17
  • There are times it would be nice even for a FileNotFound to return a null reference instead. Imagine trying to use LINQ against a set of files. If a file is removed before the iterator completes the exception would blow out the entire stack leaving you in an inconsistent state instead of just being able to do a null check and continue. – Matthew Whited Oct 26 '09 at 19:24
  • @Rex: And I would consider them different. That leads me to think that there's no "right" (ie, "more correct") answer, which was my original point ;) – Adam Robinson Oct 26 '09 at 19:26
  • @Rex M, ... even if it is an API question, the API design should not be based on the domain model? What is the Business Domain Model problem this method's return value is to be used for? At some point in the stack frame every method is eventually used to implement some business function. If the absence of a user will prevent that business function from completing sucessfully, then at some point in the stack frame, the code should throw an exception. – Charles Bretana Oct 26 '09 at 19:32
  • 2
    @Charles: You're answering the question "should an exception be thrown at some point", but the question is "should this function throw the exception". The correct answer is "maybe", not "yes". – Adam Robinson Oct 26 '09 at 19:35
  • This certainly looks like a method that would be in the DAL. There is no reason for a DAL method to concern itself with business rules. That's what the business layer is for. It should return null, and the business layer should contain the code whether to throw an exception or not. If the user not being found is a possible case or not, the the business layer should handle it. As Adam(above) says, this question is not whether an exception should be thrown somewhere. The question is whether the exception should be thrown from THIS method. My answer is no. Null will be better in my opinion – Ryan Alford Oct 26 '09 at 19:49
  • @Adam, I see the distinction you are making... Yes I agree, in principle, if an exception is to be thrwn, then where it is thrown is a separate question. For example, if multiple business processes use this method, and some of them can complete successfully with no User, and others cannot, then the exception should be throw furthur up the call stack, closer to where it intercepts with the busines Domain Model processes that require a User. But again, these distinctions should all be based on an analysis of the buisiness Domain Model, including the processes within it. – Charles Bretana Oct 26 '09 at 20:09
  • So we've all settled on "Maybe, it depends on the larger context of your application" which, coincidentally, is my answer ;) – Rex M Oct 26 '09 at 20:24
  • @Rex, yes... and I'd define "larger context of the application" so that it is clear that this is that subset or modification of the complete Business Domain Model which most closely "fits" with or describes the business functions and processes implemented by the application. – Charles Bretana Oct 26 '09 at 20:54
  • @Rex ... no. The question is whether THIS method should throw an exception. That answer should be no. It doesn't depend on any context. Now whether an exception should be thrown elsewhere, that is what depends on the "larger context of the application". But as to whether this "GetUserByID" method should throw that exception, my opinion is no. – Ryan Alford Oct 26 '09 at 22:31
  • @Eclipsed4utoo, you state your opinion w/o any justification. Why do you feel this way? Without knowing the context, how can you say whether it depends on it or not? What if the only method calling this method is a method that applies an adjustment to a financial account based upon a GUID decrypted from a value passed in from the internet? And if your answer is no, What IS your criteria for when a method should throw an exception? – Charles Bretana Oct 27 '09 at 01:09
  • @Eclipsed4utoo we don't know if this is something like an API to an LDAP or ActiveDirectory service, which would almost certainly throw an exception if the caller demands data for a specific ID which does not exist. There is broad agreement here that all are possibly correct, we don't have enough information to know for sure. – Rex M Oct 27 '09 at 01:23
  • @Charles and @Rex my justification is in my previous comment. The first comment in the code clearly states that the code accesses a database. And again, the question isn't "should an exception be thrown somewhere". That would definitely depend on business rules. The question is whether a method that gets data from a database should throw an exception if the data is not found/blank. If code accesses a database, it should only do that. It shouldn't contain business logic. It should return null, and the calling code should handle the null using the application's business rules. – Ryan Alford Oct 27 '09 at 01:44
  • (continued)... @Charles and @Rex The business layer/code should determine whether to throw an exception. Not the database code. – Ryan Alford Oct 27 '09 at 01:48
  • Business logic should not throw exceptions. Exceptions are for exceptional application circumstances - when code encounters a situation it cannot deal with. – Rex M Oct 27 '09 at 01:54
  • @Rex, well, ".. for exceptional application circumstances ..." No offense, but That's not clear or informative... "encounters a situation it cannot deal with" is little better - it eliminates all "expected" situations, cause if I expect a situation, I can ALWAYS deal with it, maybe not and keep the application running in a consistent state, but I can always "deal with It" - A better definition is that you should throw an exception "anytime the function of method cannot succssfully complete whatever function it was designed to perform." – Charles Bretana 0 s – Charles Bretana Oct 27 '09 at 19:37
  • @eclipsed4utoo, Even if the code is accesssing a database, it is still a business issue, not a technical one... No one but the business domain expert knows whether or not the fucntion of requesting a record from the database for a specific key, should be considered successful if no such record exists with that key. Clearly, for some business scenarios, this might be ok, and for some others, this might indicate a failure justifying an exception. – Charles Bretana Oct 28 '09 at 03:10
29

Personally, I use NULL. It makes clear that there is no data to return. But there are cases when a Null Object may be usefull.

Fernando
  • 4,029
  • 1
  • 26
  • 35
  • Just about to add this as an answer myself. NullObjectPattern or special case pattern. Then you could implement one for each case, NoUserEntitiesFound, NullUserEntities etc. – David Swindells Feb 01 '16 at 14:33
26

If your return type is an array then return an empty array otherwise return null.

Darin Dimitrov
  • 1,023,142
  • 271
  • 3,287
  • 2,928
  • Is 0 items in a list the same as list unassigned at this time? – AnthonyWJones Oct 26 '09 at 18:58
  • 3
    0 items in a list is not the same as `null`. It allows you to use it in `foreach` statements and linq queries without worrying for `NullReferenceException`. – Darin Dimitrov Oct 26 '09 at 19:06
  • 5
    I am surprised this hasn't been upvoted more. This seems like a pretty reasonable guideline to me. – shashi Aug 30 '12 at 12:18
  • Well, empty container is just a specific instance of the null object pattern. Which might be appropriate, we cannot tell. – Deduplicator Jul 21 '15 at 22:01
  • Returning an empty array when the data isn't available is simply *wrong*. There's a difference between the data being available and containing no items and the data not being available. Returning an empty array in both cases makes it impossible to know which is the case. Doing it just so you can use a foreach without checking whether the data even exists is beyond silly - the caller *should* have to check whether the data exists and a NullReferenceException if the caller doesn't check is *good* because it exposes a bug.. – Reinstate Monica Dec 22 '15 at 16:36
12

You should throw an exception (only) if a specific contract is broken.
In your specific example, asking for a UserEntity based on a known Id, it would depend on the fact if missing (deleted) users are an expected case. If so, then return null but if it is not an expected case then throw an exception.
Note that if the function was called UserEntity GetUserByName(string name) it would probably not throw but return null. In both cases returning an empty UserEntity would be unhelpful.

For strings, arrays and collections the situation is usually different. I remember some guideline form MS that methods should accept null as an 'empty' list but return collections of zero-length rather than null. The same for strings. Note that you can declare empty arrays: int[] arr = new int[0];

H H
  • 263,252
  • 30
  • 330
  • 514
  • Glad you mentioned that strings are different, since Google showed me this when I was deciding whether to return an empty string. – Noumenon Mar 06 '13 at 07:48
  • Strings, collections and arrays are *not* different. If MS says so MS is wrong. There is a difference between an empty string and null, and between an empty collection and null. In both cases the former represents existing data (of size 0) and the latter represents lack of data. In some cases the distinction is very important. For example, if you look up an entry in a cache, you want to know the difference between the data being cached but being empty and the data not being cached so that you must fetch it from the underlying data source, where it might not be empty. – Reinstate Monica Dec 22 '15 at 16:38
  • 1
    You seem to miss the point and context. `.Wher(p => p.Lastname == "qwerty")` should return an empty collection, not `null`. – H H Dec 22 '15 at 21:55
  • @HenkHolterman If you are able to access the full collection and apply a filter that accepts no items in the collection, an empty collection is the correct result. But if the full collection doesn't exist, an empty collection is extremely misleading - null or throwing would be correct depending on whether the situation is normal or exceptional. As your post didn't qualify which situation you were talking about (and now you clarify you are talking about the former situation) and as the OP was talking about the latter situation, I have to disagree with you. – Reinstate Monica Dec 23 '15 at 01:39
11

This is a business question, dependent on whether the existence of a user with a specific Guid Id is an expected normal use case for this function, or is it an anomaly that will prevent the application from successfully completing whatever function this method is providing the user object to...

If it's an "exception", in that the absence of a user with that Id will prevent the application from successfully completing whatever function it is doing, (Say we're creating an invoice for a customer we've shipped product to...), then this situation should throw an ArgumentException (or some other custom exception).

If a missing user is ok, (one of the potential normal outcomes of calling this function) then return a null....

EDIT: (to address comment from Adam in another answer)

If the application contains multiple business processes, one or more of which require a User in order to complete successfully, and one or more of which can complete successfully without a user, then the exception should be thrown further up the call stack, closer to where the business processes which require a User are calling this thread of execution. Methods between this method and that point (where the exception is being thrown) should just communicate that no user exists (null, boolean, whatever - this is an implementation detail).

But if all processes within the application require a user, I would still throw the exception in this method...

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Charles Bretana
  • 143,358
  • 22
  • 150
  • 216
  • -1 to the downvoter, +1 to Charles -- it is entirely a business question and there is no best practice for this. – Austin Salonen Oct 26 '09 at 18:55
  • It is crossing the streams. Whether or not it is an "error condition" is driven by the business logic. How to handle that is an application architecture decision. The business logic is not going to dictate that a null be returned, just that the requirements are satisfied. If the business is deciding method return types, then they are too involved in the technical aspect of the implementation. – Joseph Ferris Oct 26 '09 at 19:28
  • @joseph, the core principle behind "structured exception handling" is that exceptions should be thrown when methods cannot complete whatever function they were coded to implement. You are right, in that if the business function this method has been coded to implement can be "successfully completed", (whatever that means in the Domain Model), then you don't need to throw an exception, you could return a null, or a boolean "FoundUser" variable, or whatever... How you communicate to the calling method that no user was found then becomes a technical implementation detail. – Charles Bretana Oct 26 '09 at 19:37
10

I personally would return null, because that is how I would expect the DAL/Repository layer to act.

If it doesn't exist, don't return anything that could be construed as successfully fetching an object, null works beautifully here.

The most important thing is to be consistant across your DAL/Repos Layer, that way you don't get confused on how to use it.

Alex Moore
  • 3,415
  • 1
  • 23
  • 39
7

I tend to

  • return null if the object id doesn't exist when it's not known beforehand whether it should exist.
  • throw if the object id doesn't exist when it should exist.

I differentiate these two scenarios with these three types of methods. First:

Boolean TryGetSomeObjectById(Int32 id, out SomeObject o)
{
    if (InternalIdExists(id))
    {
        o = InternalGetSomeObject(id);

        return true;
    }
    else
    {
        return false;
    }
}

Second:

SomeObject FindSomeObjectById(Int32 id)
{
    SomeObject o;

    return TryGetObjectById(id, out o) ? o : null;
}

Third:

SomeObject GetSomeObjectById(Int32 id)
{
    SomeObject o;

    if (!TryGetObjectById(id, out o))
    {
        throw new SomeAppropriateException();
    }

    return o;
}
Johann Gerell
  • 24,991
  • 10
  • 72
  • 122
  • @Matt: Yes, sir, I most certainly do! Fixed. – Johann Gerell Oct 19 '10 at 12:05
  • 2
    Really, this is the only one-size-fits-all answer, and therefore the wholly and solely truth! :) Yes, it depends on the *assumptions* based on which the method is invoked... So first clear up these assumptions and then pick the right combination of the above. Had to scroll too much down to get to here :) +100 – yair Mar 25 '18 at 19:48
  • This seems to be the pattern to use, except it doesn't support async methods. I referenced this answer and added an async solution with Tuple Literals -> [here](https://stackoverflow.com/questions/1626597/should-functions-return-null-or-an-empty-object#answer-51565145) – ttugates Jul 27 '18 at 20:34
6

Yet another approach involves passing in a callback object or delegate that will operate on the value. If a value is not found, the callback is not called.

public void GetUserById(Guid id, UserCallback callback)
{
    // Lookup user
    if (userFound)
        callback(userEntity);  // or callback.Call(userEntity);
}

This works well when you want to avoid null checks all over your code, and when not finding a value isn't an error. You may also provide a callback for when no objects are found if you need any special processing.

public void GetUserById(Guid id, UserCallback callback, NotFoundCallback notFound)
{
    // Lookup user
    if (userFound)
        callback(userEntity);  // or callback.Call(userEntity);
    else
        notFound(); // or notFound.Call();
}

The same approach using a single object might look like:

public void GetUserById(Guid id, UserCallback callback)
{
    // Lookup user
    if (userFound)
        callback.Found(userEntity);
    else
        callback.NotFound();
}

From a design perspective, I really like this approach, but has the disadvantage of making the call site bulkier in languages that don't readily support first class functions.

Marc
  • 408
  • 3
  • 6
  • Interesting. When you started talking about delegates, I immediately started wondering if Lambda expressions could be used here. – 7wp Nov 08 '09 at 08:31
  • Yup! As I understand it, C# 3.0 and up's lambda syntax is basically syntactic sugar for anonymous delegates. Similarly in Java, without the nice lambda or anonymous delegate syntax, you can simply create an anonymous class. It's a bit uglier, but can be really handy. I suppose these days, my C# example could have used Func or something like it instead of a named delegate, but the last C# project I was on was still using version 2. – Marc Nov 08 '09 at 18:30
  • +1 I like this approach. A problem though is that it's not conventional and slightly increases the barrier to entry for a codebase. – timoxley Nov 11 '09 at 01:18
4

I'd say return null instead of an empty object.

But the specific instance that you have mentioned here, you are searching for an user by user id, which is sort of the key to that user, in that case I'd probably want to to throw an exception if no user instance instance is found.

This is the rule I generally follow:

  • If no result found on a find by primary key operation, throw ObjectNotFoundException.
  • If no result found on a find by any other criteria, return null.
  • If no result found on a find by a non-key criteria that may return a multiple objects return an empty collection.
Johann Gerell
  • 24,991
  • 10
  • 72
  • 122
  • Why would you throw an exception in any of these cases? Sometimes users don't exist in the database, and we expect that may not happen. It's not exceptional behavior. – siride Jul 28 '18 at 15:30
4

We use CSLA.NET, and it takes the view that a failed data fetch should return an "empty" object. This is actually quite annoying, as it demands the convention of checking whether obj.IsNew rathern than obj == null.

As a previous poster mentioned, null return values will cause code to fail straight away, reducing the likelihood of stealth problems caused by empty objects.

Personally, I think null is more elegant.

It's a very common case, and I'm surprised that people here seem surprised by it: on any web application, data is often fetched using a querystring parameter, which can obviously be mangled, so requiring that the developer handle incidences of "not found".

You could handle this by:

if (User.Exists(id)) {
  this.User = User.Fetch(id);
} else {
  Response.Redirect("~/notfound.aspx");
}

...but that's an extra call to the database every time, which may be an issue on high-traffic pages. Whereas:

this.User = User.Fetch(id);

if (this.User == null) {
  Response.Redirect("~/notfound.aspx");
}

...requires only one call.

Keith Williams
  • 2,257
  • 3
  • 19
  • 29
4

I prefer null, since it's compatible with the null-coalescing operator (??).

nobody
  • 19,814
  • 17
  • 56
  • 77
3

It will vary based on context, but I will generally return null if I'm looking for one particular object (as in your example) and return an empty collection if I'm looking for a set of objects but there are none.

If you've made a mistake in your code and returning null leads to null pointer exceptions, then the sooner you catch that the better. If you return an empty object, initial use of it may work, but you may get errors later.

Jacob Mattison
  • 50,258
  • 9
  • 107
  • 126
  • +1 I was questioning the same logic you are saying here, which is why i posted the question to see what others opinions about this would be – 7wp Oct 26 '09 at 18:58
3

The best in this case return "null" in a case there no a such user. Also make your method static.

Edit:

Usually methods like this are members of some "User" class and don't have an access to its instance members. In this case the method should be static, otherwise you must create an instance of "User" and then call GetUserById method which will return another "User" instance. Agree this is confusing. But if GetUserById method is member of some "DatabaseFactory" class - no problem to leave it as an instance member.

Kamarey
  • 10,832
  • 7
  • 57
  • 70
  • Can I ask why I would want to make my method static? What if I want to use Dependency Injection ? – 7wp Oct 26 '09 at 19:13
  • Ok now I get your logic. But I stick with the Repository pattern, and I like to use dependency injection for my repositories therefore I can't use static methods. But +1 for suggesting to return null :) – 7wp Oct 26 '09 at 19:21
3

In our Business Objects we have 2 main Get methods:

To keep things simple in the context or you question they would be:

// Returns null if user does not exist
public UserEntity GetUserById(Guid userId)
{
}

// Returns a New User if user does not exist
public UserEntity GetNewOrExistingUserById(Guid userId)
{
}

The first method is used when getting specific entities, the second method is used specifically when adding or editing entities on web pages.

This enables us to have the best of both worlds in the context where they are used.

Mark Redman
  • 24,079
  • 20
  • 92
  • 147
3

I personally return a default instance of the object. The reason is that I expect the method to return zero to many or zero to one (depending on the method's purpose). The only reason that it would be an error state of any kind, using this approach, is if the method returned no object(s) and was always expected to (in terms of a one to many or singular return).

As to the assumption that this is a business domain question - I just do not see it from that side of the equation. Normalization of return types is a valid application architecture question. At the very least, it is subject for standardization in coding practices. I doubt that there is a business user who is going to say "in scenario X, just give them a null".

Joseph Ferris
  • 12,576
  • 3
  • 46
  • 72
  • +1 I like the alternative view of the problem. So basically you are saying whichever approach I choose should be fine as long as the method is consistent throughout the application? – 7wp Oct 26 '09 at 19:33
  • 1
    That is my belief. I think consistency is extremely important. If you do things multiple ways in multiple places, it introduces a higher risk for new bugs. We personally have gone with the default object approach, because it works well with the Essence pattern that we use in our domain model. We have a single generic extension method that we can test against all domain objects to tell us if it is populated or not, so that we know that any DO can be tested with a call of objectname.IsDefault() - avoiding any equality checks directly. – Joseph Ferris Oct 26 '09 at 20:06
3

I'm a french IT student, so excuse my poor english. In our classes we are being told that such a method should never return null, nor an empty object. The user of this method is supposed to check first that the object he is looking for exists before trying to get it.

Using Java, we are asked to add a assert exists(object) : "You shouldn't try to access an object that doesn't exist"; at the beginning of any method that could return null, to express the "precondition" (I don't know what is the word in english).

IMO this is really not easy to use but that's what I'm using, waiting for something better.

Saend
  • 31
  • 1
  • 1
    Thanks you for your answer. But I dislike the idea of checking first if it exists. The reason is that generates an additional query to the database. In a application that is being accessed by millions of people in a day this can result in dramatic performance loss. – 7wp Oct 28 '09 at 20:51
  • 1
    One benefit is that the check for existence is appropriately abstract: if (userExists) is slightly more readable, closer to the problem domain, & less 'computery' than: if (user == null) – timoxley Nov 11 '09 at 01:23
  • And I would argue that 'if (x == null)' is a decades old pattern that if you haven't seen it before, you haven't written code for very long (and you should get used to it as it is in millions of lines of code). "Computery"? We are talking about database access... – Lloyd Sargent Dec 01 '14 at 17:10
3

If the case of the user not being found comes up often enough, and you want to deal with that in various ways depending on circumstance (sometimes throwing an exception, sometimes substituting an empty user) you could also use something close to F#'s Option or Haskell's Maybe type, which explicitly seperates the 'no value' case from 'found something!'. The database access code could look like this:

public Option<UserEntity> GetUserById(Guid userId)
{
 //Imagine some code here to access database.....

 //Check if data was returned and return a null if none found
 if (!DataExists)
    return Option<UserEntity>.Nothing; 
 else
    return Option.Just(existingUserEntity);
}

And be used like this:

Option<UserEntity> result = GetUserById(...);
if (result.IsNothing()) {
    // deal with it
} else {
    UserEntity value = result.GetValue();
}

Unfortunately, everybody seems to roll a type like this of their own.

yatima2975
  • 6,580
  • 21
  • 42
2

I typically return null. It provides a quick and easy mechanism to detect if something screwed up without throwing exceptions and using tons of try/catch all over the place.

whatsisname
  • 5,872
  • 2
  • 20
  • 27
2

For collection types I would return an Empty Collection, for all other types I prefer using the NullObject patterns for returning an object that implements the same interface as that of the returning type. for details about the pattern check out link text

Using the NullObject pattern this would be :-

public UserEntity GetUserById(Guid userId)

{ //Imagine some code here to access database.....

 //Check if data was returned and return a null if none found
 if (!DataExists)
    return new NullUserEntity(); //Should I be doing this here instead? return new UserEntity();  
 else
    return existingUserEntity;

}

class NullUserEntity: IUserEntity { public string getFirstName(){ return ""; } ...} 
vikram nayak
  • 591
  • 5
  • 14
2

To put what others have said in a pithier manner...

Exceptions are for Exceptional circumstances

If this method is pure data access layer, I would say that given some parameter that gets included in a select statement, it would expect that I may not find any rows from which to build an object, and therefore returning null would be acceptable as this is data access logic.

On the other hand, if I expected my parameter to reflect a primary key and I should only get one row back, if I got more than one back I would throw an exception. 0 is ok to return null, 2 is not.

Now, if I had some login code that checked against an LDAP provider then checked against a DB to get more details and I expected those should be in sync at all times, I might toss the exception then. As others said, it's business rules.

Now I'll say that is a general rule. There are times where you may want to break that. However, my experience and experiments with C# (lots of that) and Java(a bit of that) has taught me that it is much more expensive performance wise to deal with exceptions than to handle predictable issues via conditional logic. I'm talking to the tune of 2 or 3 orders of magnitude more expensive in some cases. So, if it's possible your code could end up in a loop, then I would advise returning null and testing for it.

Jim L
  • 2,297
  • 17
  • 20
2

Forgive my pseudo-php/code.

I think it really depends on the intended use of the result.

If you intend to edit/modify the return value and save it, then return an empty object. That way, you can use the same function to populate data on a new or existing object.

Say I have a function that takes a primary key and an array of data, fills the row with data, then saves the resulting record to the db. Since I'm intending to populate the object with my data either way, it can be a huge advantage to get an empty object back from the getter. That way, I can perform identical operations in either case. You use the result of the getter function no matter what.

Example:

function saveTheRow($prim_key, $data) {
    $row = getRowByPrimKey($prim_key);

    // Populate the data here

    $row->save();
}

Here we can see that the same series of operations manipulates all records of this type.

However, if the ultimate intent of the return value is to read and do something with the data, then I would return null. This way, I can very quickly determine if there was no data returned and display the appropriate message to the user.

Usually, I'll catch exceptions in my function that retrieves the data (so I can log error messages, etc...) then return null straight from the catch. It generally doesn't matter to the end user what the problem is, so I find it best to encapsulate my error logging/processing directly in the function that gets the data. If you're maintaining a shared codebase in any large company this is especially beneficial because you can force proper error logging/handling on even the laziest programmer.

Example:

function displayData($row_id) {
    // Logging of the error would happen in this function
    $row = getRow($row_id);
    if($row === null) {
        // Handle the error here
    }

    // Do stuff here with data
}

function getRow($row_id) {
 $row = null;
 try{
     if(!$db->connected()) {
   throw excpetion("Couldn't Connect");
  }

  $result = $db->query($some_query_using_row_id);

  if(count($result) == 0 ) {
   throw new exception("Couldn't find a record!");
  }

  $row = $db->nextRow();

 } catch (db_exception) {
  //Log db conn error, alert admin, etc...
  return null; // This way I know that null means an error occurred
 }
 return $row;
}

That's my general rule. It's worked well so far.

2

Interesting question and I think there is no "right" answer, since it always depends on the responsibility of your code. Does your method know if no found data is a problem or not? In most cases the answer is "no" and that's why returning null and letting the caller handling he situation is perfect.

Maybe a good approach to distinguish throwing methods from null-returning methods is to find a convention in your team: Methods that say they "get" something should throw an exception if there is nothing to get. Methods that may return null could be named differently, perhaps "Find..." instead.

Marc Wittke
  • 2,991
  • 2
  • 30
  • 45
  • +1 I like the idea to use a uniform naming convention for signaling to the programmer how that function is to be consumed. – 7wp Oct 28 '09 at 21:19
  • 1
    Suddenly I recognize that this is what LINQ does: consider First(...) vs. FirstOrDefault(...) – Marc Wittke Oct 28 '09 at 22:09
2

If the object returned is something that can be iterated over, I would return an empty object, so that I don't have to test for null first.

Example:

bool IsAdministrator(User user)
{
    var groupsOfUser = GetGroupsOfUser(user);

    // This foreach would cause a run time exception if groupsOfUser is null.
    foreach (var groupOfUser in groupsOfUser) 
    {
        if (groupOfUser.Name == "Administrators")
        {
            return true;
        }
    }

    return false;
}
Jan Aagaard
  • 10,940
  • 8
  • 45
  • 80
2

I like not to return null from any method, but to use Option functional type instead. Methods that can return no result return an empty Option, rather than null.

Also, such methods that can return no result should indicate that through their name. I normally put Try or TryGet or TryFind at the beginning of the method's name to indicate that it may return an empty result (e.g. TryFindCustomer, TryLoadFile, etc.).

That lets the caller apply different techniques, like collection pipelining (see Martin Fowler's Collection Pipeline) on the result.

Here is another example where returning Option instead of null is used to reduce code complexity: How to Reduce Cyclomatic Complexity: Option Functional Type

Zoran Horvat
  • 10,924
  • 3
  • 31
  • 43
  • 1
    I wrote an answer, I can see it is similar to yours as I scrolled up, and I agree, you can implement an option type with a generic collection with 0 or 1 elements. Thanks for the additional links. – Gabriel P. May 17 '17 at 10:04
1

More meat to grind: let's say my DAL returns a NULL for GetPersonByID as advised by some. What should my (rather thin) BLL do if it receives a NULL? Pass that NULL on up and let the end consumer worry about it (in this case, an ASP.Net page)? How about having the BLL throw an exception?

The BLL may be being used by ASP.Net and Win App, or another class library - I think it is unfair to expect the end consumer to intrinsically "know" that the method GetPersonByID returns a null (unless null types are used, I guess).

My take (for what it's worth) is that my DAL returns NULL if nothing is found. FOR SOME OBJECTS, that's ok - it could be a 0:many list of things, so not having any things is fine (e.g. a list of favourite books). In this case, my BLL returns an empty list. For most single entity things (e.g. user, account, invoice) if I don't have one, then that's definitely a problem and a throw a costly exception. However, seeing as retrieving a user by a unique identifier that's been previously given by the application should always return a user, the exception is a "proper" exception, as in it's exceptional. The end consumer of the BLL (ASP.Net, f'rinstance) only ever expects things to be hunky-dory, so an Unhandled Exception Handler will be used instead of wrapping every single call to GetPersonByID in a try - catch block.

If there is a glaring problem in my approach, please let me know as I am always keen to learn. As other posters have said, exceptions are costly things, and the "checking first" approach is good, but exceptions should be just that - exceptional.

I'm enjoying this post, lot's of good suggestions for "it depends" scenarios :-)

Mike Kingscott
  • 477
  • 1
  • 5
  • 19
  • And of course, today I've come across a scenario where I'm going to return NULL from my BLL ;-) That said, I could still throw an exception and use try / catch in my consuming class BUT I still have a problem: how does my consuming class know to use a try / catch, similar how do they know to check for NULL? – Mike Kingscott Nov 05 '09 at 09:25
  • You can document that a method throws an exception via the @throws doctag and you'd document the fact it can return null in the @return doctag. – timoxley Nov 11 '09 at 01:11
1

I think functions should not return null, for the health of your code-base. I can think of a few reasons:

There will be a large quantity of guard clauses treating null reference if (f() != null).

What is null, is it an accepted answer or a problem? Is null a valid state for a specific object? (imagine that you are a client for the code). I mean all reference types can be null, but should they?

Having null hanging around will almost always give a few unexpected NullRef exceptions from time to time as your code-base grows.

There are some solutions, tester-doer pattern or implementing the option type from functional programming.

Gabriel P.
  • 3,400
  • 2
  • 32
  • 23
0

I am perplexed at the number of answers (all over the web) that say you need two methods: an "IsItThere()" method and a "GetItForMe()" method and so this leads to a race condition. What is wrong with a function that returns null, assigning it to a variable, and checking the variable for Null all in one test? My former C code was peppered with

if ( NULL != (variable = function(arguments...)) ) {

So you get the value (or null) in a variable, and the result all at once. Has this idiom been forgotten? Why?

no comprende
  • 139
  • 1
  • 1
0

I agree with most posts here, which tend towards null.

My reasoning is that generating an empty object with non-nullable properties may cause bugs. For example, an entity with an int ID property would have an initial value of ID = 0, which is an entirely valid value. Should that object, under some circumstance, get saved to database, it would be a bad thing.

For anything with an iterator I would always use the empty collection. Something like

foreach (var eachValue in collection ?? new List<Type>(0))

is code smell in my opinion. Collection properties shouldn't be null, ever.

An edge case is String. Many people say, String.IsNullOrEmpty isn't really necessary, but you cannot always distinguish between an empty string and null. Furthermore, some database systems (Oracle) won't distinguish between them at all ('' gets stored as DBNULL), so you're forced to handle them equally. The reason for that is, most string values either come from user input or from external systems, while neither textboxes nor most exchange formats have different representations for '' and null. So even if the user wants to remove a value, he cannot do anything more than clearing the input control. Also the distinction of nullable and non-nullable nvarchar database fields is more than questionable, if your DBMS is not oracle - a mandatory field that allows '' is weird, your UI would never allow this, so your constraints do not map. So the answer here, in my opinion is, handle them equally, always.

Concerning your question regarding exceptions and performance: If you throw an exception which you cannot handle completely in your program logic, you have to abort, at some point, whatever your program is doing, and ask the user to redo whatever he just did. In that case, the performance penalty of a catch is really the least of your worries - having to ask the user is the elephant in the room (which means re-rendering the whole UI, or sending some HTML through the internet). So if you don't follow the anti-pattern of "Program Flow with Exceptions", don't bother, just throw one if it makes sense. Even in borderline cases, such as "Validation Exception", performance is really not an issue, since you have to ask the user again, in any case.

Community
  • 1
  • 1
Oliver Schimmer
  • 387
  • 2
  • 14
0

An Asynchronous TryGet Pattern:

For synchronous methods, I believe @Johann Gerell's answer is the pattern to use in all cases.

However the TryGet pattern with the out parameter does not work with Async methods.

With C# 7's Tuple Literals you can now do this:

async Task<(bool success, SomeObject o)> TryGetSomeObjectByIdAsync(Int32 id)
{
    if (InternalIdExists(id))
    {
        o = await InternalGetSomeObjectAsync(id);

        return (true, o);
    }
    else
    {
        return (false, default(SomeObject));
    }
}
ttugates
  • 5,818
  • 3
  • 44
  • 54
-1

You should be throwing an exception if it is an exceptional circumstance that you call that code with an invalid user ID. If it is NOT an exceptional circumstance, then what you are essentially doing is using a "getter" method to test whether a user exists or not. That is like trying to open a file to see if it exists or not (lets stick to c#/java here) instead of using the exists method, or trying to access dictionary elements and seeing if they exist by looking at the return value instead of using the "contains" method first.

Therefore, it is likely you are after an additional method such as "exists" to first check if there is such a user. Performance of exceptions is definitely not a reason to just not use them at all unless you have genuine performance issues.

BobTurbo
  • 891
  • 3
  • 11
  • 14
  • 1
    I'm convinced that in this particular case it is better to return a Null. Please see the accepted answer and also read the comments. – 7wp Jan 12 '11 at 13:15
  • 1
    Also exceptions should not be thrown for expected cases. Like in my example where it is expected that sometimes the user ID you are looking for may not be found. Because if you use exceptions for expected cases, and those cases happen many times in a short period of time, then the overhead of exception handling can actually ding you a bit in performance. Like say in a long running loop. – 7wp Jan 12 '11 at 13:18