I feel like Ayende, posting little snippets of code weirdness as you write them (or see them). Today, when testing the OrderService and transactional stuff for the MVC Storefront, I wanted to write a test that would conclusively show that an order would fail if the Authorize() method of IPaymentService threw. You may think this is a given, but some errant try/catch blocks could destroy this assumption rather quickly…
Many Ways – Which is Correct?
I could have made a credit card number that didn’t pass Luhn validation and passed that into the IPaymentService and indeed, I would have failure. But it doesn’t pinpoint WHERE I want it to fail (in the Authorize() method).
I could hardcode some values, mess with the test a bit (hack, and more hack) – but the answer came when thinking about IPaymentService and what it’s supposed to do (Authorize(), Refund(), and Capture()). And what it’s NOT supposed to do. And thus was born my ThrowPaymentService (this may be obvious to some – I find it hysterical):
public class ThrowPaymentService:IPaymentService { public Commerce.MVC.Data.Transaction Authorize(Commerce.MVC.Data.Order order) { throw new InvalidOperationException("Authorization failed"); } public Commerce.MVC.Data.Transaction Refund(Commerce.MVC.Data.Order order) { throw new InvalidOperationException("Refund failed"); } public void Capture(Commerce.MVC.Data.Order order) { throw new InvalidOperationException("Capture failed"); } }
A completely useless class in every sense of the word “useless”. However, in my case, it exactly nails what I need it to (authorization failure) and helps my test to pass. Writing Fail to make a test work. Sensational.
The question I have at this point is:
Is it more correct to throw when the authorization is declined, or is it better to return null?
The answer, to me, seemed pretty obvious and play’s by a core rule in programming:
If a method can’t do what it’s supposed to, throw.
Throwing an exception allows you to also notify the consumer of this method what went wrong, and you could include a descriptive message as returned from the payment gateway if you like. Ultimately, the calling method needs to decide what to do when an exception happens – not the routine itself.
But then I read this code over and there’s a part of me that’s dead inside; as if I’ve wasted a precious few minutes of my time on earth. I’m not sure but I think this is the first truly useless code I’ve ever written (in general terms – but I’m sure there are others that will disagree). But it works perfectly, it’s what I needed, so it’s full of Win!
