tags:

views:

246

answers:

5

I've just come across a property setter that catches exceptions (all Exceptions; I know that's bad, but it's not relevant here), and only logs them. First of all, I think it should through them again as well; why wait for a crash and a log study when you can know something is wrong right away?

However, my main question is, do I validate against invalid date values, add a RuleViolation object to a ValidationRules object on my document, or throw an InvalidDate exception, or just let the CLR throw the exception for me (invalid dates are nothing but invalid dates, not checked for range etc.)

A: 

Catching and rethrowing is the worst thing to do. Its expensive to TRY, if youre just going to rethrow what the point? You can catch unhandled exceptions with the global.asax for example if you need to log them.

In terms of validation, from a web perspective i always use regex validators for dates, these fire client and server side so i know when im inside an

if(Page.IsValid)

block that my txtDate.Text is a valid date, so i dont bother checking because its just wasteful.

Andrew Bullock
+4  A: 

It depends on the specific task at hand. If you are writing a library class that will be used as a component in other programs and the contract of the method of the class says that it should only accept valid dates then throwing the Exception is fine.

If you are accepting user input and then waiting for an exception is a bad practice. In this case you should validate the date yourself.

Exceptions are for exceptional cases, and should not be part of your logic. It usually means that a contract was broken by the programmer.

Vincent Ramdhanie
+1  A: 

It really depends on the logic of your application. Exceptions should only really be thrown for circumstances that are exceptional. For something like validation it depends on the tolerance for invalid data.

When you are building an interractive application and the user may enter anything, it is probably ok that the document gets into an invalid state and you should expose validation information via properties on the document class.

If you are processing pre-prepared documents from a database or log file then it probably isn't ok for the document to be invalid and continuing to operate after that might corrupt data in the system. When that happens you should throw.

Wolfbyte
+2  A: 

I think it depends where the date values come from. If it comes from user input or some other source where it is perfectly well possible to enter 'invalid' dates, then validation would be the way to go. On the other hand, if there is no foreseeable reason why the data values might be invalid, then throwing an exception is appropriate.

Daan
+1  A: 

Exceptions should be thrown whenever the method or class member is unable to complete whatever task it is designed to accomplish.

So for a property setter, if the setter is unable to set the property, then it should throw an exception.

As to whether you should catch it and rethrow it, the answer is yes, but only if you need to process the exception immediately in the setter, before passing it up the stack... but logging it is not a reason to do that. In general, you should implement cross-cutting logging of exceptions at a higher level, where the exception is NOT going to be re-thrown... if you are taking care of those cross-cutting concerns higher up the stack somewhere, then no, definitely do not catch and re-throw the same exception.

However, if you are writing a tool, or a framework library, where you want your component's clients to have a clearly defined set of expected exceptions, and you have defined your own custom exceptions that your component will throw to client code, and which client components will expect to see, then you may want to catch CLR generated exceptions and rethrow your own custom exceptions instead.. Always include the Actual underlying exception in your custom exceptions "InnerException" property before passing it up the stack, so that the data in it is available to whatever system end up consuming it.

Charles Bretana