views:

211

answers:

8

I'll explain what I mean by input error checking.

Say you have a function doSomething(x).

If the function completes successfully doSomething does something and returns nothing. However, if there are errors I'd like to be notified. That is what I mean by error checking.

I'm looking for, in general, the best way to check for errors. I've thought of the following solutions, each with a potential problem.

  1. Flag error checking. If doSomething(x) completes successfully return null. Otherwise, it returns a boolean or an error string. Problem: Side effects.

  2. Throwing an exception. Throw an exception if doSomething(x) encounters an error. Problem: If you are performing error checking for parameters only, throwing an IllegalArgumentExceptionseems inappropriate.

  3. Validating input prior to function call. If the error checking is only meant for the parameters of the function, then you can call a validator function before calling the doSomething(x) function. Problem: What if a client of the class forgets to call the validator function before calling doSomething(x)?

I often encounter this problem and any help or a point in the right direction would be much appreciated.

+1  A: 

I decide which method to use usually on the type of interface.

  • User interface (GUI): I validate before calling business methods, because the user wants to know what was wrong.

  • On technical interfaces between components or systems, the interface should have been tested and work properly in this case I throw exceptions.

stacker
You should do both: client side **and** server side validation (don't trust the client!)
Pascal Thivent
+5  A: 

Throw an exception is the best way.

If you are performing error checking for parameters only, throwing an IllegalArgumentException seems inappropriate.

Why? That's the purpose of this Exception.

Daniel Engmann
Agreed, why would that be inappropriate? It is an exceptional condition (I'm assuming), and the error is specific enough your client code can do something meaningful with it.
lucas
I should have clarified that the parameter error checking I had in mind was user input. From Effective Java: "Use runtime exceptions to indicate programming errors". As aiobee mentions below, runtime exceptions are for bugs rather than invalid user input.
Jack
You don't have to let the runtime exception trickle all the way up to the UI. You can trap it by the calling function and instead of re-throwing it, print a user-friendly message. For example, `tan(90 deg)` is undefined and the function should thrown an exception, but the calling function should convert the caught exception into a message for the user.
FrustratedWithFormsDesigner
I understand your point. But have people often looked for a checked exception version of IllegalArgumentException? Do they create their own? The unchecked part is what bothers me about it.
Jack
It is inappropriate indeed RuntimeExceptions should be used to indicate programming errors. This should be a checked exception.
OscarRyz
@Support, it does indeed indicate a programming error if the input to the method should have already been sanitized. You aren't passing unsanitized user input directly into unrelated methods... are you?
Mike Daniels
Makes sense, but I think Jacks is doing some kind of library.. mmmh ... and want to enforce clients to perform the validation. May be may be :P
OscarRyz
+1  A: 

Exceptions is the way to go. Your stated problem with exceptions can be mitigated by the proper implementation of exception throwing / handling. Use exceptions to your advantage by validating parameters at the lowest level that you need them and throwing an exception if the validation fails. This allows you to avoid redundantly checking for validity at multiple levels in the call stack. Throw the exception at the bottom and let the stack unroll to the appropriate place for handling the error.

bshields
+3  A: 

Your second suggestion ("Throwing an exception") is the best choice. The other two options rely on the invoker either doing something before ("Validating input prior to function call") or after ("Flag error checking") the call to the method. Either way, the extra task is not mandated by the compiler so someone invoking the function isn't forced to call the "extra thing" so problems are not spotted till run-time.

As for "Throwing an Exception" and your suggested 'problem', well the answer is throw appropriate exception types for the code. If the input parameters are invalid, then throw an InvalidArgumentException (since that's the appropriate error). If the exception is for functionality (e.g. cannot open network connection), use another exception type or create your own.

Phil
My reputation is too low to add comments to other people's answers, but I agree with "stacker"s first bullet point (for User Interfaces).
Phil
+4  A: 
  1. Flag error checking

This is appropriate in some cases, depending on what you mean by an "error".

An example from the API: If you try to add an object to a Set, which already contains another object which equals the new object, the add method sort of "fails" and indicates this by returning false. (Note that we're on a level at which it's technically not even an "error"!)

2.Throwing an exception

This is the default option.

Question is now, should you go for a checked exception (which you need a throws declaration or try/catch clauses) or an unchecked exception (an exception that extends RuntimeException). There are a few rules of thumbs here.

From Java Practices -> Checked versus unchecked exceptions:

  • Unchecked exceptions: Represent defects in the program (bugs) - often invalid arguments passed to a non-private method.

  • Checked exceptions: Represent invalid conditions in areas outside the immediate control of the program (invalid user input, database problems, network outages, absent files)

Note that IllegalArgumentException is an unchecked exception, perfectly suitable for throwing when arguments are not as they should be.

If you want to throw a checked exception, you could A) roll your own by extending Exception, B) use some existing checked exception or C) "chain" a runtime exception in, for instance, an IOException: throw new IOException(new IllegalArgumentException("reason goes here..."));

3.Validating input prior to function call

Relying on the fact that the client should have sanitized / checked his arguments before the call seems like a bad idea to me.

aioobe
+1  A: 

The method you choose depends on the situation, and they are not mutually exclusive so you can mix them all in the same solution (although whether that's a good idea really depends on your situation).

  1. Choose this method if you want a very simple method for handling errors. This method might be OK for situations where the calling function can accept any value the called function returns. There might be situations where business logic dictates this as an OK choice, such as returning a specific message string when a resource cannot be properly located, or a server does not respond. Generally, I don't use this or see this technique in Java very much, as exceptions are a better mechanism for error handling.

  2. Throw an exception when your function runs into un defined behaviour. If you have a math function that can only operate on positive integers and someone passes -1, you should thrown an InvalidArguementException. If your function is given the ID of a product in a database, but the product cannot be found by a query, you could throw a custom ProductNotFound exception.

  3. Validating input is a good idea, I would say it should be done by the called function, rather than the caller - unless the caller can avoid an exception from the callee by validating the input before passing it. If you work in a language that supports Design By Contract, validating input would be done as the function's precondition.

I usually use #2 and #3. I haven't written code with error flags for a while. The exception to that might be a function that returned an enum, where one possible value indicated an error code. That was driven more by a business rule than anything else.

And generally, try to keep it simple.

FrustratedWithFormsDesigner
+2  A: 

I agree with throwing exceptions. I want to add another option that combines #2 and #3 - the proxy pattern. That way your code stays fairly cohesive - validation in one place and business logic in another. This makes sense if you have a large set of calls that need to be validated.

Create a proxy to handle validation. Have it delegate all calls to the actual implementation of your business logic interface after it validates, otherwise it can throw exceptions if something does not validate.

lucas
A: 

Throw a custom checked exception.

 doSomething(WithX x ) throws BusinessRuleViolatedException 
OscarRyz