But I do the null check all the time, and then skip the "Save", but have logic in the 'else' to do something like create an actual customer account.
The problem with the solution provided is that you get an exception, whether you can deal with it in the body or not. And unlike Lisp, you're going to unwind the stack at that point (or be in the catch). All in all, not happy with the proposed solution.
Actually you don't want to ever use exception driven development. The use of the as operator is a simple metadata check. Throwing exceptions in .Net is expensive.
Just because I felt the need to check my own facts (Friday afternoon?) I wrote a simple console app with two classes Class1 and Class2 : Class1. Using a Stopwatch object I checked the elapsed ticks for an 'as' an 'is' and an invalid cast exception check.
'as' = 5 ticks
'is' = 4 ticks
'invalid cast' = 147990 ticks
Note this was just enough test to validate my assumption and that I never condone my developers taking an optimize first development approach... But there is still a right and a wrong way to do the job sometimes and exception driven development is not the right way.
> Actually you don't want to ever use exception driven development. The use of the as operator is a simple metadata check. Throwing exceptions in .Net is expensive.
it doesn't matter, it's throwing an exception when the caller code is broken. That's ok, it's an error.
I'm not sure what you mean by exception driven development.
The original code was trying to express that e.NewItem should be a Customer. In all the following code, it is still an error for e.NewItem to not be a Customer. The question is what should happen when that error occurs.
There are three options discussed: 1) NullReferenceException, 2) Silent error, 3) InvalidCastException. Performance does not come into it, since if any of the error handling code paths get triggered, there is a bug which needs to be fixed. The claim in the article is that by throwing the right exception (InvalidCastException) it will be easier to track down the bug than if the wrong exception was thrown, or if the error was ignored.
Some clarification seems to be required since I am being voted into oblivion (perhaps never ever use ever or never?:) )
When writing some code that relies on a method in a specific class or interface being present then actively test for it and react appropriately.
Writing the code based on how it is hoped the program state will be and letting the exceptions fall out is what I mean by exception driven development. In many cases this is a code smell that indicates a section that was not completely thought out.
The next step with an invalid cast exception or a type check is to take action and do something about it. When faced with an exception there can be more ambiguity about it's source especially when there are many lines in the try block or if you are calling into other libraries.
What if there is some code in the save method that throws the exception for type casting and the invalid cast exception assumes that the source is the base class problem? Now you can add further debug time to the cost of trying to short circuit the type check that was intended up front.
Unfortunately, the real problem here is a broken API. A statically typed language whose standard library includes APIs that require downcasting is pretty embarrassing.
In general though, if the situation is truly exceptional (wrong type being passed) then counting the ticks used throwing the exception should be compared against the ongoing cost of the increased complexity to the codebase to handle the exception in another fashion (e.g. an if/else block).
Also, if you're just comparing the ticks of as+is VS invalid cast wouldn't you want to multiply each by the # of times they'll be executed? The as/is would be executed EVERY time the function in question was called. The invalid cast would, in this case, likely be executed once or twice ever and then the bug that is causing an InvalidCastException would be fixed.
That's part of it. But the author should articulate it.
There is also certain (specific) situations where the original code (e.NewItem as Customer).Save() would fail but ((Customer)e.NewItem).Save() wouldn't. That specific case is where the type of e.NewItem has an implicit cast to Customer (as would return null, but a (Customer) cast would work).
But if you're doing implicit casting you might be facing other issues.
Yeah I no argument that it's good to know what your code does but what you choose to do is context specific, why would throwing InvalidCastException be better than NullRef, neither tell me why the failure happened.
On a somewhat unrelated note why is this trivial C# 101 lesson on the bloody front-page. I've got to imagine maybe 1 out of 10 readers care and far fewer didn't already know this.
InvalidCastException include the class name, doesn't it? That makes it order of magnitude easier to track down where it came from. The NullRef just tells you that it MAY have been that the 'as' failed, or it could have been a bona-fide NullRef.