Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

We're discussing the best way to handle errors in calls to a method.

We have a credit model where we allow users to deduct credits for certain actions. We'd like to make a call to:

User.DeductCredits(10);

Then the Deduct method will check things like

  1. Does the user exist?
  2. Is the user active?
  3. Does the user have at least 10 credits?

If not, this result should fail.

We're currently writing this just as a class library, but trying to keep it open to convert to a web service in the future. The options we've discussed so far are:

  1. Throw an exception when an error occurs.
  2. Create a new Result object that would have T has the type of data expected back, and wrapping that with a status code and message.

We didn't like option #1, because then code to call deduct credits would look something like:

try
{
   user.DeductCredits(10);
}
catch(InvalidUserException e)
{
}
catch(InactiveUserException e)
{
}
catch(CreditBalanceException e)
{
}

Additionally, if we turn this in to a service, we can't force exceptions on the user.

The main reason we didn't like option #2 was that it seemed to be returning more data than what we need. For example:

  var address = user.GetMailAddress();

Now address has the status code and message, as well as a data property, or value property. So to get the city, you'd have code like:

if(!address.HasError)
   state = address.Data.State;

as opposed to:

state = address.State;

So is there a best practice for this type of situation? Is one option really any better than the other?

share|improve this question
2  
You could also go with something like return_code = user.GetMailAddress(user_address); where the desired values are returned in an out parameter and the function return is always the error code (likely 0 for success). –  David Thornley Dec 20 '11 at 19:04
    
Yeah, we talked about that, but it felt too much like a Win32 API from back in the day. It opens the door for people to start including more out parameters to a method on a whim. –  taylonr Dec 20 '11 at 19:06
    
Removed tag:Rest as I don't see the connection to a REST architecture here. –  sdg Dec 20 '11 at 20:35
add comment

3 Answers

up vote 5 down vote accepted

The best practice is a combination of the two: if it is a real error, (something that should not really happen,) then you should throw an exception. If it is not a real error, but something that may happen within the regular course of things, then the function should return some sort of a result enum. In your case, without knowing the specifics and just guessing from the names I see, it seems to me that "invalid user" is an error, so an exception should be thrown, while "inactive user" and "insufficient funds" are probably not errors, but results.

I assume that by "user" you mean the programmer who will be using your service. In my book, "we can't force exceptions on the user" is just plain dead wrong. You must force exceptions on the user; as a matter of fact, the user expects exceptions when things go wrong. That's the way things are done in this millennium. Also, do not worry, you will very rarely, if ever, need to write multiple catch statements for all possible exceptions. Try it, and you will see that in actual usage scenarios things do not work that way.

share|improve this answer
    
What I meant by "we can't force exceptions on the user" is if it's a web service, the exception wouldn't transfer from the DLL to the consumer. Because it's possible the consumer is using some other language that doesn't know about .NET exceptions. In those cases they'd come up as a 500 internal server error, but that's not particularly helpful. –  taylonr Dec 20 '11 at 19:52
    
Oh, I see, OK. Well then, enum result codes all the way. –  Mike Nakis Dec 20 '11 at 19:53
    
That's how we're going forward. Just wanted to make sure we weren't missing some 3rd case that made a lot of sense. –  taylonr Dec 20 '11 at 19:57
1  
Exceptions in .NET get translated into SOAP Faults which most WS-I clients are equipped to handle. –  Mike Brown Dec 20 '11 at 21:00
add comment

I vote for number 1: throw exceptions.

And no, your code shouldn't try to capture all possible exceptions. You should create code to avoid most of those exceptions if you can handle it. Instead of your example, consider this:

if (user.Credits >= 10) { user.DeductCredits(10); }

When your application arrives at this part of the code you should already be certain that the user exists otherwise what would there be in that user variable?

share|improve this answer
    
True the user should exist, that was a bad case. The reason I don't like the checking before hand is, now every consumer of the API has to do this check. It's also moving the responsibility from the user to know if it CAN deduct credits, to the consumer. For example, what if you can only deduct credits, if the user has been in the system 30 days, now you have user.IsInSystem30Days && user.Credits >= 10 etc. Even moving that behind a user.CanDeductCredits() becomes a problem because the state could change between those 2 lines of code. –  taylonr Dec 20 '11 at 19:57
2  
Also, in a multithreaded environment, the check may succeed, and by the time control reaches the function, the situation may have changed. The only way to avoid this is to add some locking or transactioning mechanism, which may be an overkill if you can just get away by returning an error. –  Mike Nakis Dec 20 '11 at 20:10
add comment

I've been handling this by going ahead and throwing exceptions where warranted, and I've had my API (which happens to be used over JSON, not web services, but it shouldn't matter) always return an object that has an error property, the message number of the original call, and the return value.

The client I'm using to consume this makes the call through a layer that strips the real response out of the returned object, and throws an exception on the client if the error property has a value. So once that client layer is written the experience of how exceptions work is pretty natural. Maybe this would work for your prospective API users.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.