0

Suppose I have a method that must return an object (say from a database layer) and takes as input some info about the requested object.

In the normal case, the method should return the object. But what happens if the precondition of the methods are not fulfilled by the caller; how should I (the code writer of the method) inform the caller?

Take for example the case that I should return information about the user from a db, and I take as input the username and the password. In the normal case I should return the User object. But what if the username and password are null, if they mismatch, if the password is too short... how should I inform the caller what is exactly the problem? Normally I would have thrown an exception, but here - When to throw an exception? was suggested that it is a bad idea. Should I put an errorDetail field in the User, or make a method checkInputData or getErrorStatus...

Community
  • 1
  • 1
Random42
  • 8,989
  • 6
  • 55
  • 86
  • Throw an exception (...drum roll...), when something exceptional (i.e. unexpected) happens, such as parameters not meeting the method's contract. – Mitch Wheat Mar 26 '12 at 15:38
  • Retrieveing an inexisting User from a DB can not possibly be specified in a contract; hou else to return this information to the method's caller? – Random42 Mar 26 '12 at 16:59

3 Answers3

1

It sounds perfectly reasonable to throw an exception in this case. The method, which should be doing one thing and only one thing, is simply trying to fetch a user record from the database. If a precondition fails, give up. If the database errors out, give up. If no user is found, give up. Let the calling code deal with the consequences.

An exception is a perfectly acceptable exit path for a method.

In the case of the precondition, it's not the method's responsibility to fix that. It's the calling code's responsibility. So throw an ArgumentException of some kind and let the calling code deal with it.

In the case of a database error, either catch it and throw a custom exception (something like an InfrastructureFailed exception) for the application to handle accordingly. Basically the application needs to tell the user that there are technical difficulties and to please try again later.

In the case of no user being found, that sounds like a failed login. Throw some kind of SecurityException and let the calling application handle it by notifying the user that the login has failed. (Don't be more specific. For example, don't tell the user that the username was fine but the password was bad. That's giving a malicious user more information than you want them to have. Just say that the login failed, nothing more.)

You also mention a case of a password being too short. I imagine that's something that should be validated before it gets to this point. In this case, that falls up under input checking for the method. So the preconditions fail and the method never even tries to get to the database. But "password is too short" probably isn't a good error to tell a user at a login prompt. Rather, that's for when they try to create the password.

One thing you shouldn't do is return null or an empty user object or anything like that. This puts the onus on the calling code to check if there was an error. Exceptions are a perfectly valid way of notifying calling code of an error. A "magic object" being returned is just like any other "magic string" in code, but worse because it leaks out of the method and into other code.

Now, this isn't always a complete rule, of course. It depends on the structure of the application. For example, if this is a web service or some other kind of request/response system then you might want to have a standard response object which would indicate failure. That assumes some application context around the method, though. And in that case you probably should still be dealing with multiple methods. The inner one (a domain logic method) would throw the exception, the outer one (an application UX-aware method) would catch the exception and craft an appropriate non-null response to the UI.

How you handle the exception is up to the logical structure of your application. But throwing and catching exceptions (keep in mind that "catching" is not the same thing as meaningfully "handling") is perfectly acceptable behavior.

David
  • 208,112
  • 36
  • 198
  • 279
  • Thanks for the elaborate answer. I don't know if I should open another question for this: suppose I have to do CRUD operations, and the read operation I do it as you mentioned (with exceptions). What about the other 3 operations which should be void theoreticaly? Should I throw exception also, and be consistent or should I return an object holding information about the possible errors (or OK status)? – Random42 Mar 26 '12 at 16:49
  • @m3th0dman: I'm not sure what you mean. You're performing all of these operations in a single transaction? If so, the transaction should be aborted if there's an error so that there are no partially-completed consequences (leaving things in an unknown state). You may also want to look into the Unit Of Work pattern. – David Mar 26 '12 at 17:15
  • Assume that autoCommit flag iis true and each operation is performed in a single transaction. – Random42 Mar 26 '12 at 21:13
0

Does it make sense to return some kind of sentinel value? If so, consider using that. Otherwise, why not throw an exception?

Marcin
  • 48,559
  • 18
  • 128
  • 201
  • It does not make sense to return a sentinel value. Throwing an exception it's what I would do but I mentioned that I have read (link in the question) suggestions that is a bad idea to throw an exception in cases such as an invalid password is given for returning a user. – Random42 Mar 26 '12 at 16:28
0

I would return an object that contains both a user (null in the case of an error), a success flag (false in the case of an error) and a list of strings as validation messages (on error, a list of reasons why it failed).

Adding an errorDetail field on the user object doesn't make sense since that is not really a property of the user object. It's just part of this return data from the method. This path leads to objects with muddles properties that are only used by certain methods.

Trent
  • 1,280
  • 11
  • 12
  • So you create a Wrapper Class that holds holds the User as a Field, and other information about the eventual errors? Suppose you have a strongly typed language and is not suitable to return an array of elements having different types. – Random42 Mar 26 '12 at 16:25
  • Yeah... I would create a wrapper class that would represent the return data from this method - user object, success flag and message strings. What language are you working in? I've never worked in a language that wouldn't support something like that. – Trent Mar 26 '12 at 20:22
  • One advantage to this approach as opposed to throwing an exception is you can provide more than one message back to the user. (i.e. zip code is invalid and user name is already taken) – Trent Mar 26 '12 at 20:24
  • I'm working in Java; it's legal but not suitable to send a List of Objects. Anyway, having a wrapper complicates things because you cannot use polymorhism for example. And about more information, youn can put as many fields as you want in a custom Exception. – Random42 Mar 26 '12 at 21:11