6

I have a few functions on Data access layer

public Order RetrieveById(int id)
public List<Order> RetrieveByStatus(OrderStatus status)

Now i am bit confuse on exception raising.

In case of RetrieveById function, id which is less than 1 is an invalid id therefore i feel like raising an exception. And i feel like returning null for the Id which doesn't exist in the database. Then it feels like i am over complicating.

In case of RetrieveByStatus, i feel like returning a empty list when there is no data in the database for that status.

However i have seen that some people raising an exception when RetrieveById cannot return anything but then RetrieveByStatus should not raise exception when there is no record or should it?

Could anyone please clarify these concepts for me?

Anthony Pegram
  • 123,721
  • 27
  • 225
  • 246
user1767363
  • 193
  • 2
  • 6
  • 1
    One other possibility is to use the `Order RetrieveByID(id)` and `bool TryRetrieveByID(id, out Order)` pattern. – McGarnagle Oct 26 '12 at 04:13

4 Answers4

3

I would prefer to return null in the first case and an empty list in the second case.

But if you want to raise exception then You can raise exception for public Order RetrieveById(int id) because it means that id is not valid as calling the first method means that you know the id and the it needs to be there.

In the second case the OrderStatus might be valid and there is not record found against it so returning an empty list is a good idea.

Asif Mushtaq
  • 13,010
  • 3
  • 33
  • 42
3
  1. Read MSDN first: Creating and Throwing Exceptions (C# Programming Guide). It lists both situations when you are expected to throw an exception, and when to avoid it.
  2. Also take into account What is the real overhead of try/catch in C#?

In any case you'll have to process either null return or an exception thrown


As for myself, I'd prefer in both your methods not to throw exception explicitly. I'd say, there is nothing bad, if your method returns null, if it failed to find an object by id. Whereas the RetrieveByStatus method could return an empty collection, not null.

Besides you could follow the pattern used in LINQ, where you have, say, Enumerable.First and Enumerable.FirstOrDefault methods (either throwing an exception or returning null), so you could use the appropriate one in a certain situation, when the id is 100% valid or when on the contrary it could be missing. While methods returning a sequence of elements don't throw exceptions if the sequence to return appears to be empty; consider Enumerable.Where.

Community
  • 1
  • 1
horgh
  • 17,918
  • 22
  • 68
  • 123
  • Thanks for nice links and explanation. After reading all the comments, i am more inclined to raise exception in first case and empty list in 2nd case. – user1767363 Oct 26 '12 at 08:46
3

In the first case i would possibly go for a exception and handle myself,instead of returning null.What if your first method is used in a way that the returned object is saved to a Order reference.There is a very high chance of NullReferenceException being thrown,when someone tries to call a method or property on that object.

For the second method i would go for a empty list as some have suggested.

Prabhu Murthy
  • 9,031
  • 5
  • 29
  • 36
-1

I like to avoid returning null whenever possible, because NullRefExceptions are much more cryptic than a specific exception, say OrderNotFoundException. Also, code gets pretty obtuse when you have to constantly expect entities to be null. This ought to be an exception case anyway -- where did you get that id if it doesn't exist in the db?

On the cases you suspect this is more likely to throw an error, you could add a DoesObjectExist or TryGet type method (or even extension method).

tallseth
  • 3,635
  • 1
  • 23
  • 24
  • You would force the code which call that method to catch the exception if there is a alternative flow when the element doesn't exist. I really prefer return null and allow to decide what to do with that. – august0490 Sep 09 '20 at 15:23