3
\$\begingroup\$
 internal void ShiftSwapInstrument(SwapCurve shiftedCurve, string swapToShift)
 {
    if (SwapExists(shiftedCurve, swapToShift))
    {
       var rateToShift = shiftedCurve.Swaps.Single(r => r.Description() == swapToShift);
       rateToShift.Rate = rateToShift.Rate + 1 / 10000.0;
       return;
    }
    throw new PdhSwapRateNotFoundException(swapToShift);
 }

 private bool SwapExists(SwapCurve shiftedCurve, string swapToShift)
 {
    var swap = Array.Find(shiftedCurve.Swaps, r => r.Description() == swapToShift);
    return swap == null ? false : true;
 }

The above code is what I've written to handle processing when an item is not found in an array. The exception should 'never' be thrown - it's truly exceptional if it does happen - thus I can use Single without reservation within the if-block.

My question is whether my exception handling is correct/feasible when items are not found in my array and is it correct for me to return; when I do?

Secondary question

I noted the following about Array.Find on MSDN

Return Value

Type: T The first element that matches the conditions defined by the specified predicate, if found; otherwise, the default value for type T.

Am I correct in my understanding, that since SwapCurve & Swaps are reference types the default value will be null?

\$\endgroup\$
  • \$\begingroup\$ Your code looks too complicated, look to shorter version in my answer, doing exaclty the same. \$\endgroup\$ – Artur Mustafin Oct 7 '11 at 9:33
  • \$\begingroup\$ @ArturMustafin - complicated, how so? an excessive call or two is perfectly acceptable to me if it increases readability and understandability. You are correct, in that your version is shorter, but that extra second it takes me to understand the intent of the method six months from now to me is worth the 'complicated' code \$\endgroup\$ – Ahmad Oct 7 '11 at 9:38
1
\$\begingroup\$

Your error handling looks correct to me as long as your application is catching PdhSwapRateNotFoundException at some point and logging appropriate warning/error to notify user about this.

A slight cosmetic modification to your code could be like:

{
    if (!SwapExists(shiftedCurve, swapToShift))
        throw new PdhSwapRateNotFoundException(swapToShift);

    var rateToShift = shiftedCurve.Swaps.Single(r => r.Description() == swapToShift);
    rateToShift.Rate = rateToShift.Rate + 1 / 10000.0;
    return; // you don't actually need this.
}

And you're correct default value of reference type is null.

Edit: I assume shiftedCurve.Swaps will not be null, otherwise Find will throw an exception. And if you think it could be the case then you may want to update your code to check for shiftedCurve.Swaps == null and handle it appropriately.

\$\endgroup\$
  • 1
    \$\begingroup\$ Cosmetic change looks good \$\endgroup\$ – Ahmad Oct 7 '11 at 9:24
  • \$\begingroup\$ What about readability and performance? and i think that call to SwapExists leads to excessive searches twice: first time in ShapExists call, another when the single actual value is changed. You can easily combine these two calls in one call to SingleOrDefault<T>() \$\endgroup\$ – Artur Mustafin Oct 7 '11 at 9:30
  • \$\begingroup\$ re the Edit - Find returns the default value which is null. Swaps will never be null at this point \$\endgroup\$ – Ahmad Oct 7 '11 at 9:31
  • \$\begingroup\$ @ArturMustafin - Your suggestions makes sense, but the original question was about error handling. You downvoted my answer :-(, np, mate! \$\endgroup\$ – Paresh Oct 7 '11 at 9:41
  • \$\begingroup\$ Array.Find is much faster then SingleOrDefault, so when talking about performance you are wrong! \$\endgroup\$ – Paresh Oct 7 '11 at 9:58
1
\$\begingroup\$

My thoughts:

Step1:

You'd better to improve readability of you code since the IL will be exacly the same:

Step2:

Remove excessive method calls by using extenstion method .SingleOrDefault<T>(..):

 internal void ShiftSwapInstrument(SwapCurve shiftedCurve, string swapToShift) 
 { 
    var rateToShift = shiftedCurve.Swaps.SingleOrDefault(r => r.Description() == swapToShift)
    if (rateToShift == null) 
    { 
        throw new PdhSwapRateNotFoundException(swapToShift); 
    }
    rateToShift.Rate = rateToShift.Rate + 1 / 10000.0; 
 } 
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.