How to avoid duplication of code when using two constructors in a class C#-CodePudding

Hi All I have two request DTOs, of which one is the parent and one is the child.

    public class PaymentSupplierPaymentRequest
{
    [JsonProperty("description")]
    public string Description { get; set; }

    [JsonProperty("amount")]
    [Required]
    public decimal Amount { get; set; }

    [JsonProperty("currency")]
    public string Currency { get; set; }

    [JsonProperty("supplierid")]
    [Required]
    public string SupplierId { get; set; }

    [JsonProperty("accountid")]
    [Required]
    public long AccountId { get; set; }
}

public class PaymentSupplierLitePaymentRequest: PaymentSupplierPaymentRequest
{
    [JsonProperty("merchantCode")]
    [Required]
    public string MerchantCode { get; set; }
}

There are two types of payment request one which the Merchant Code is required and one where the merchant code is not required. Depending on the request I have another class that uses two constructors to handle the data passed into the request.

    private readonly string _merchantCode;
    private readonly string _installationId;
    private readonly string _version;
    private readonly string _paymentMethodMaskCode;
    private readonly string _orderDescription;
    private readonly long _amount;
    private readonly string _currency;
    private readonly string _email;
    private const byte Exponent = 2;
    private readonly string _orderCode;

        public GemPayPaymentRequestBuilder(PaymentSupplierLitePaymentRequest paymentRequest)
    {
        _amount = (long)(paymentRequest.Amount * (decimal)Math.Pow(10, Exponent));
        _orderDescription = paymentRequest.Description;
        _currency = paymentRequest.Currency;
        _orderCode = $"{paymentRequest.AccountId}_{DateTime.UtcNow.Ticks}";
        _merchantCode = paymentRequest.MerchantCode;
    }

    public GemPayPaymentRequestBuilder(PaymentSupplierPaymentRequest paymentRequest, string email, WorldPayMerchantConfig config)
    {
        _amount = (long)(paymentRequest.Amount * (decimal)Math.Pow(10, Exponent));
        _orderDescription = paymentRequest.Description;
        _currency = paymentRequest.Currency;
        _orderCode = $"{paymentRequest.AccountId}_{DateTime.UtcNow.Ticks}";
        _email = email;
        _merchantCode = config.MerchantCode;
        _installationId = config.InstallationId;
        _version = config.Version;
        _paymentMethodMaskCode = config.PaymentMethodMaskCode;
    }

The problem is as you can see in the code I am duplicating the initialising of certain local variables in both constructors.

        _amount = (long)(paymentRequest.Amount * (decimal)Math.Pow(10, Exponent));
        _orderDescription = paymentRequest.Description;
        _currency = paymentRequest.Currency;
        _orderCode = $"{paymentRequest.AccountId}_{DateTime.UtcNow.Ticks}";

Does anyone have a cleaner way of doing this? I have tried using constructor inheritance for the second constructor as shown below.

       public GemPayPaymentRequestBuilder(PaymentSupplierLitePaymentRequest paymentRequest)
    {
        _amount = (long)(paymentRequest.Amount * (decimal)Math.Pow(10, Exponent));
        _orderDescription = paymentRequest.Description;
        _currency = paymentRequest.Currency;
        _orderCode = $"{paymentRequest.AccountId}_{DateTime.UtcNow.Ticks}";
        _merchantCode = paymentRequest.MerchantCode;
    }

    public GemPayPaymentRequestBuilder(PaymentSupplierPaymentRequest paymentRequest, string email, WorldPayMerchantConfig config) : this(paymentRequest)
    {
        _email = email;
        _merchantCode = config.MerchantCode;
        _installationId = config.InstallationId;
        _version = config.Version;
        _paymentMethodMaskCode = config.PaymentMethodMaskCode;
    }

But this does not work as using : this(paymentRequest) throws a conversion error as it cannot convert from PaymentSupplierPaymentRequest to PaymentSupplierLitePaymentRequest. Any Ideas?

CodePudding user response：

Using constructors only.

The simplest way to do this is to call : this from different constructor (as PaymentSupplierLitePaymentRequest inherits PaymentSupplierPaymentRequest, not vice versa), but you need to handle other params in second constructor and change your code a bit:

public GemPayPaymentRequestBuilder(PaymentSupplierLitePaymentRequest paymentRequest) : this(paymentRequest, null, null)
{
    _merchantCode = paymentRequest.MerchantCode;
}

public GemPayPaymentRequestBuilder(PaymentSupplierPaymentRequest paymentRequest, string email, WorldPayMerchantConfig config)
{
    _amount = (long)(paymentRequest.Amount * (decimal)Math.Pow(10, Exponent));
    _orderDescription = paymentRequest.Description;
    _currency = paymentRequest.Currency;
    _orderCode = $"{paymentRequest.AccountId}_{DateTime.UtcNow.Ticks}";

    _email = email;

    if (config != null)
    {
        _merchantCode = config.MerchantCode;
        _installationId = config.InstallationId;
        _version = config.Version;
        _paymentMethodMaskCode = config.PaymentMethodMaskCode;
    }
}

Or another approach - you can create non-public constructor to initialize common data and call it from public ones (the code is cleaner (my opinion) than in approach above and you have separate place to initialize common properties without nulls and ifs):

// protected (only for common data) and PaymentSupplierPaymentRequest as parameter
protected GemPayPaymentRequestBuilder(PaymentSupplierPaymentRequest paymentRequest) 
{
    _amount = (long)(paymentRequest.Amount * (decimal)Math.Pow(10, Exponent));
    _orderDescription = paymentRequest.Description;
    _currency = paymentRequest.Currency;
    _orderCode = $"{paymentRequest.AccountId}_{DateTime.UtcNow.Ticks}";
}

public GemPayPaymentRequestBuilder(PaymentSupplierLitePaymentRequest paymentRequest) : this((PaymentSupplierPaymentRequest)paymentRequest)
{
    _merchantCode = paymentRequest.MerchantCode;
}

public GemPayPaymentRequestBuilder(PaymentSupplierPaymentRequest paymentRequest, string email, WorldPayMerchantConfig config) : this(paymentRequest)
{
    _email = email;
    _merchantCode = config.MerchantCode;
    _installationId = config.InstallationId;
    _version = config.Version;
    _paymentMethodMaskCode = config.PaymentMethodMaskCode;
}

Using separate method.

You can create separate method which will contain logic which is same for both types:

private void InitializeCommonData(PaymentSupplierPaymentRequest paymentRequest)
{
    _amount = (long)(paymentRequest.Amount * (decimal)Math.Pow(10, Exponent));
    _orderDescription = paymentRequest.Description;
    _currency = paymentRequest.Currency;
    _orderCode = $"{paymentRequest.AccountId}_{DateTime.UtcNow.Ticks}";
}

Then you can initialize properties/fields:

public GemPayPaymentRequestBuilder(PaymentSupplierLitePaymentRequest paymentRequest)
{
    this.InitializeCommonData(paymentRequest);
    _merchantCode = paymentRequest.MerchantCode;
}

public GemPayPaymentRequestBuilder(PaymentSupplierPaymentRequest paymentRequest, string email, WorldPayMerchantConfig config)
{
    this.InitializeCommonData(paymentRequest);
    _email = email;
    _merchantCode = config.MerchantCode;
    _installationId = config.InstallationId;
    _version = config.Version;
    _paymentMethodMaskCode = config.PaymentMethodMaskCode;
}

CodePudding user response：

You can pass always the base class and invoke always to your complete constructor:

    public GemPayPaymentRequestBuilder(PaymentSupplierPaymentRequest paymentRequest)
        : this(paymentRequest, null, null)
    {
    }

    public GemPayPaymentRequestBuilder(PaymentSupplierPaymentRequest paymentRequest, string email, WorldPayMerchantConfig config)
    {
        _amount = (long)(paymentRequest.Amount * (decimal)Math.Pow(10, Exponent));
        _orderDescription = paymentRequest.Description;
        _currency = paymentRequest.Currency;
        _orderCode = $"{paymentRequest.AccountId}_{DateTime.UtcNow.Ticks}";
            
        _email = email;

        var liteRequest = paymentRequest as PaymentSupplierLitePaymentRequest;
        if (liteRequest != null)
        {
            _merchantCode = liteRequest.MerchantCode;
        }
        else if (config != null)
        {
            _merchantCode = config.MerchantCode;
        }

        if (config != null)
        {
            _installationId = config.InstallationId;
            _version = config.Version;
            _paymentMethodMaskCode = config.PaymentMethodMaskCode;
        }
    }
}

In the complete constructor (the constructor that manage all possible parameters) you can try cast your class. If the class is Lite, you set it's properties.