I’m writing a class to interface with a simple hardware device over a COM port. The device can configured to use various modes, so my class has a SetOperatingMode
function, that takes in an enum
of type UsbDeviceMode
. It looks something like this:
class UsbDevice
{
public void SetOperatingMode(UsbDeviceMode mode)
{
byte[] buffer = new byte[4];
buffer[0] = 0x5A;
buffer[1] = 0x02;
buffer[2] = (byte)mode;
buffer[3] = 0x00; //IO_TYPE is always 0 in this case.
_port.Write(buffer, 0, 4);
int read = _port.Read(buffer, 0, 2);
bool successfulSet = (read == 2 && buffer[0] == 0xFF && buffer[1] == 0x00);
}
}
enum UsbDeviceMode
{
IO_MODE = 0x00,
IO_CHANGE = 0x10,
I2C_S_20KHZ = 0x20,
I2C_S_50KHZ = 0x30,
I2C_S_100KHZ = 0x40,
I2C_S_400KHZ = 0x50,
I2C_H_100KHZ = 0x60,
I2C_H_400KHZ = 0x70,
I2C_H_1000KHZ = 0x80,
SPI_MODE = 0x90,
SERIAL = 0x01
};
There is the distinct possibility that this operation might fail due to any number of reasons: The COM port might no longer exist, the device might have locked up or failed, or for whatever reason, the operation failed.
A failure would be unexpected, but not uncommon. There’s two distinct modes of failures: The COM port throws an exception (TimeoutException
and InvalidOperationException
being the expected ones). Or I could read back a failure indicator the device.
In any case, if SetOperatingMode()
fails, then the device or the communication is broken somehow, and this class can’t do anything about it.
I have 2 questions:
- Should I “pre-throw” the
InvalidOperationException
if the port is closed? From the MSDN documentation,SerialPort.Write
andSerialPort
read will throw if the port is closed. I can check that at the very top of the function, or I can just let_port.Write()
throw it. - Should there be an entirely new exception type thrown when
successfulSet
isfalse
? IfsuccessfulSet
isfalse
, there’s nothing that this class can do to. Should there be some sort ofSetOperatingModeFailedException
exception to distinguish between the COM port failing or the device failing? It seems rather time consuming to create an entire exception class just for this one spot.
Use a custom exception when you want users to be able to programmatically distinguish between certain error conditions. If that situation does not exist, you can throw a more “general” exception, and avoid creating the custom exception class.
In the specific case of your SetOperatingMode()
example, unless you need to call out specific ways that this method call might fail, you’re better off using a more general exception. In other words, if it’s your intention to throw a SetOperatingModeFailedException
exception as a possible result of calling SetOperatingMode()
, but not to programmatically distinguish what kind of operating mode failure occurred, then you can dispense with creating a custom exception (since it’s the only one that might be thrown), and simply throw an InvalidOperationException
, which is probably the closest existing exception.
If you still want to create a custom exception, create one that is reusable across different methods, like OperationFailedException
.
Creating a class is easy. It’s not a time consuming process at all. Debugging code which hides the problems is hard. And time consuming.
When deciding to create an exception or not the question you should be asking yourself is “is this behaviour normal, expected behaviour, or is this behaviour exceptional”.
In this case, the expected behaviour is that the operating mode is always set. Therefore, I would suggest that exceptions need to be thrown. I would allow any exceptions from the write operation bubble up. I would also create a SetOperatingModeFailedException if the last line reveals an error has occurred.
In this case your method really only has the responsibility for trying to set the operating mode. It doesn’t have the responsibility to manage the connection. That’s someone else’s responsibility and if it hasn’t been done properly, an exception should be thrown.
5