views:

198

answers:

8

I am working on a library designed to communicate (over RS232 serial communication) with external devices. I was thinking about error handling strategy and exceptions seemed to be right and industry standard way of reporting errors.

So I read few guidelines on exceptions. One pretty clearly states that I should not worry about performance hit:

Do not use error codes because of concerns that exceptions might affect performance negatively.

Other told me NOT to throw exception in normal cases:

Do not use exceptions for normal or expected errors, or for normal flow of control.

I am not able to draw clear line between normal/expected and other cases. For example in my library, a operation may fail because:

  1. There is no response from device. (no cable connected, device not turned on, wrong baud rate)
  2. Operation request is rejected by device because it couldn't authenticate the request.
  3. Communication failed in between. (someone tripped over the cable, device was powered off suddenly).

I can think all above as expected problems because they can happen in practice very often (infact many marketing morons call me to solve the ^problem^ in my software only to find out they didnt connect the cable to their laptop). So may be exceptions should not be thrown because otherwise application programmer will have to catch those at plenty of places (a lot of catch blocks are also NOT nice to have I believe).

On the other hand, I also tend to think that these are all errors which I somehow need to report to application programmer, and exception seems to be the way to do that. If I don't use exceptions, I will need to report these problems using some error code or error enums. (ugly, I know).

Which approach do you think I should take?

+1  A: 

With RS232, unless you have hardware handshaking enabled (and most of the time, people don't), you just won't see any more data coming in from the line. There's no way for you to tell if a device is even connected, other than the fact that nothing is being sent to the PC.

I'd classify 1 and 3 together as a RS232TimeoutError, and 2 as an RS232AuthenticationError, probably.

Generally speaking, a TimeoutError indicates that the remote device has locked up or just isn't connected. An authentication error is kind of a ProtocolError, but subtly different in that the communication is fine, but the remote device "just said no" to creating a connection with the PC.

I think setting those as exceptions is well-justified: you would never expect a timeout error during normal operations, nor would you expect an authentication error.

Mark Rushakoff
The library is intended to be used for communication with many devices (32 devices at once) and it has to be as fast as possible because it will be used in production environment. Do you think raising the exception too often will affect the performance?
Hemant
If it has to be that fast to the point where throwing exceptions is a problem, you need to write the library in C. If you raise a timeout exception, you don't immediately go back and talk to the hardware again and see another exception -- you ignore that device until you're explicitly told to go back to it.
Mark Rushakoff
@Hemant: flip the question around. If errors are happening all of the time, do you think that that will affect performance? Do you expect errors to be happening frequently at all? Will the performance hit due to using an exception be significant compared with the performance hit involved in recovering from the error at the next level of abstraction?
Stephen C
+5  A: 

You are developing a library, a component that will be utilized by other applications.

Therefore in the expected cases you mention I would certainly use exceptions to communicate to the calling application that something is amiss. You should define a custom exception for each of these scenarios and then clearly document when they may occur.

This then allows the application client code to make the decision as to how best to proceed. Only the client application can make this decision and clearly documented exceptions greatly aid this.

The best thing about a custom exception is that you can provide multiple meaningful / useful pieces of data relating to the problem/exception. This data is also nicely encapsulated in one object. Compare this to error codes and return values.

Performance can be an issue but only when the exception is thrown within a tight loop or some other high activity situation. To avoid this you could also apply a pattern used by the .NET Framework, namely provide Try....() methods (eg TryParse()) that return boolean indicating if an action succeeded or failed.

Either way I would go with custom exceptions initially, and then do performance testing to actually see if there are certain parts of the library that may need optimization.

Ash
+1  A: 

Whether or not you throw an exception is a consequence of the function's type.

  • If the function returns an X and you fail to determine a valid X, throw an exception.
  • If the function is an action (eg. Connect) and you fail to complete the action, throw an exception.
  • If the function is of the TryX variety, don't throw an exception.

So I guess I'm saying you should push the problem from "Should I throw an exception?" to "What methods will the people calling my library want?" with the caveat that the exceptions you throw should be obvious based on the methods you provide.

Strilanc
+3  A: 

I would use exceptions, in the following approach (inspired by design-by-contract)

  • Where possible, provide boolean inspection functions telling you whether an operation can be safely applied
  • for the given operation, consider such an inspection function as a precondition: if it holds, the operation can be done safely, if not, you throw an exception.

In this way, if the API user can code his key logic using if-then-else structures.

If unexpected situations arise (due to subtle timing issues, e.g.) an exception will be thrown: the developer can catch this exception and deal with it. But note: this need not be at the place where the method was invoked: it can be higher / earlier in the call stack, at a central place where all strange exceptions are handled

I have done some work on the automated analysis of error handling code in multi-million lines of C programs. These were based on coding standards requiring hand-written inspection and propagation of error codes. It turns out developers don't like writing such code, they forget it, and they easily make mistakes in it. In fact, we found 2 deviations of the coding standards (2 faults, one might say) per 1000 lines of C code.

In summary: (1) I'd use boolean inspectors (2) exceptions can be caught at places higher in the call stack; (3) relying on error codes is unsafe in practice.

Kaka Woef
Design by contract for the win!
Adrian
+1  A: 

Do not use exceptions for normal or expected errors, or for normal flow of control.

Within your method implementations, avoid purposely causing an exception in order to change execution flow, handle special logic, special cases, or handle normal or expected errors. For example, exception handling should be removed in the following function. (It is handling normal or expected errors, but as the note says Convert.ToString is not really going to fail.) There is a minor performance hit because of the time needed to "setup" exception handling within the method. It is not a significant hit, yet if you are calling this function in a loop, then it may become significant. If this method is in a library then let any exceptions bubble up to the user of the library. (Custom exceptions are different, see Ash's answer.)

Public Function Nz(ByVal value As String, ByVal valueIfNothing As String) As String
    Try
        Dim sValue As String = System.Convert.ToString(value) 'using Convert.ToString on purpose
        If IsNothing(sValue) Then
            Return valueIfNothing 
        Else
            Return sValue
        End If
    Catch ex As Exception
        'Convert.ToString handles exceptions, but just in case...
        Debug.Fail("Nz() failed. Convert.ToString threw exception.")
        Return String.Empty
    End Try
End Function

Here is a "better" implementation of the method:

Public Function Nz(ByVal value As String, ByVal valueIfNothing As String) As String
    Dim sResult As String = String.Empty
    Dim sValue As String = System.Convert.ToString(value) 'using Convert.ToString on purpose
    If IsNothing(sValue) Then
        sResult = valueIfNothing 
    Else
        sResult = sValue
    End If
    Return sResult
End Function
AMissico
+1  A: 

Do not use error codes because of concerns that exceptions might affect performance negatively.

Avoid designing everything as a function that returns true/false with "out" parameters just to avoid "imagined" performance concerns with using exceptions.

AMissico
A: 

The choice doesn't have to be between throwing an exception, or returning an error code. Try returning an exception object in an exceptional condition. The performance hit is not in creating an exception, but in throwing it. The caller can then check for a "null" return value. The returned exception could be a function return, or one of several "out" parameters.

Scott
+1  A: 

Exceptions were designed for exactly that - exceptional circumstances. I try and think of it like this - if you assume everything that you depend on is working as normal for the majority of the time, and method X fails, then it's an exceptional circumstance because it doesn't normally happen and you should define exceptions to catch the situation.

So in your situation, you assume the device to be up and running. Therefore exceptional circumstances in this case are the device not accepting a connection, it rejecting a connection, it not accepting data you send it or you not receiving data it should be sending etc. If your devices are routinely turned off several times a day, then you expect them to be turned off, so use return codes or a "bool IsDeviceOn();" method to check it before you connect.

If it's something that you do expect to happen in normal circumstances, such as querying a device for its capabilities but the one you want isn't available, then use return codes, or a bool method - e.g. "bool DoesDeviceHaveThisCapability();" Don't catch an exception when it doesn't.

Another example (for GUI apps) is user input. Don't use exceptions for that, because you do expect a user to input something incorrectly - we're not perfect.

I have experienced massive performance issues due to using exceptions when they weren't truly exceptional circumstances. One example was when I was processing a 2GB data file 2-3 times a day. Some lines had valid prices in the format 0.00. Some didn't. I used a FormatException to catch those that didn't.

Two years later when I had the chance to get a performance analyser on it, that particular line that caught the exception was responsible for 80% of the time used to analyse the file. I changed that to use the TryParse() method of int instead, and got a massive speed increase.

For your examples, I would probably use:

  1. No response from device - exception if devices should be on 24/7, return code if routinely powered off
  2. Operation unauthorised - exception
  3. Comms failed - exception
Andy Shellam
I ended up doing pretty much exactly as u suggested. Thanks.
Hemant