Is there a way to refactor the c# code having multiple conditions?

Question

Is there a better way to refactor my below code where I have many conditions in if and else statement

The reason for refactoring is to make code cleaner and readable

    public static PaymentOptions GetPaymentOptions_Auto(TestConfigurationCDO testConfiguration, int siteId)
{
    var paymentOptions = new PaymentOptions();

    var paymentOptionList = SitePaymentRepository.GetSitePaymentInfoBySiteId(
        testConfiguration,
        siteId);

    var lowestPriority = paymentOptionList.Min(x => x.Priority);
    var paymentAuto = paymentOptionList.Where(x => x.Priority == lowestPriority).FirstOrDefault();
    
    if (paymentAuto.PaymentType == PaymentMethod.Klarna)
    {
        paymentOptions = new KlarnaOptions();
    }
    else if (paymentAuto.PaymentType == PaymentMethod.PayPalDirect ||
        paymentAuto.PaymentType == PaymentMethod.Braintree ||
    paymentAuto.PaymentType == PaymentMethod.BankTransfer || paymentAuto.PaymentType == PaymentMethod.AdyenDropIn)
    {
        paymentOptions = new ClientCheckoutOptions()
        {
            paymentMethod = paymentAuto.PaymentType
        };
    }
    else if (paymentAuto.PaymentType == PaymentMethod.PayPalExpress)
    {
        paymentOptions = new PaypalOptions();
    }
    else
    {
        paymentOptions = new PaypalOptions();
    }
    return paymentOptions;
}

Answer 1

1. Extract the statements into methods. Like: paymentAuto.IsKlarnaPayment()

   This would shorten the if statements a bit.

2. At the end you have

   else if (paymentAuto.PaymentType == PaymentMethod.PayPalExpress)
   {
       paymentOptions = **new PaypalOptions()**;
   }
   else
   {
       paymentOptions = **new PaypalOptions()**;
   }
   

   This returns the same PaymentOption. So it would be better to remove the last else if statement.

3. paymentAuto can be null in this scenario (FirstOrDefault()). This can lead to null reference exception.

4. You can combine this:

   paymentOptionList.Where(x => x.Priority == lowestPriority).FirstOrDefault();

   into

   paymentOptionList.FirstOrDefault(x => x.Priority == lowestPriority);

Answer 2

You've got lots of tests for paymentAuto.PaymentType there, and many of the cases are the same. A switch statement will neaten that up, as it only names paymentAuto.PaymentType once, and it encourages you to put all of the cases nicely aligned together in code:

public static PaymentOptions GetPaymentOptions_Auto(TestConfigurationCDO testConfiguration, int siteId)
{
    var paymentOptionList = SitePaymentRepository.GetSitePaymentInfoBySiteId(
        testConfiguration,
        siteId);

    var lowestPriority = paymentOptionList.Min(x => x.Priority);
    var paymentAuto = paymentOptionList.First(x => x.Priority == lowestPriority);

    PaymentOptions paymentOptions;
    
    switch (paymentAuto.PaymentType)
    {
        case PaymentMethod.PayPalDirect:
        case PaymentMethod.Braintree:
        case PaymentMethod.BankTransfer:
        case PaymentMethod.AdyenDropIn:
            paymentOptions = new ClientCheckoutOptions()
            {
                paymentMethod = paymentAuto.PaymentType
            };
            break;

        case PaymentMethod.Klarna:
            paymentOptions = new KlarnaOptions();
            break;

        case PaymentMethod.PayPalExpress:
        default:
            paymentOptions = new PaypalOptions();
            break;
    }
}

The initial value you were assigning to paymentOptions wasn't used, so you can leave that variable uninitialised. The compiler will moan at you if you forget to initialise it somewhere in the switch statement.

That .FirstOrDefault() shouldn't fail to find a matching item. To make sure of this, I changed it to .First(), which throws an exception if it can't find a match. .Where(...).First() is the same as .First(...) (but a bit more expensive), so I changed that as well.

Annoyingly there's no .MinBy method in Linq (there are plenty of examples of implementations around, though). If you have this, you can write var paymentAuto = paymentOptionList.MinBy(x => x.Priority).