HiveBrain v1.2.0
Get Started
← Back to all entries
principlecsharpMinor

Does this ugly code fit into a design pattern?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisuglyintodesigndoescodefitpattern

Problem

I'm trying to figure out a pattern to best accommodate this pretty ugly code:

var userDiscounts = _discountRepository.GetUserDiscountsForUsersWithDiscounts(new List { buyerUserId, sellerUserId }).ToList();

// Buyer.
var buyerHasFreeCommission = false;
var buyerDiscounts = userDiscounts.Where(ud => ud.UserId == buyerUserId);
foreach(var userDiscount in buyerDiscounts)
{
    switch (userDiscount.Discount.Type)
    {
        case DiscountTypes.SingleFreeCommisionPerUser:
            buyerHasFreeCommission = true;
            break;
    }
}

var sellerHasFreeCommission = false;
var sellerDiscounts = userDiscounts.Where(ud => ud.UserId == sellerUserId);
foreach(var userDiscount in sellerDiscounts)
{
    switch (userDiscount.Discount.Type)
    {
        case DiscountTypes.SingleFreeCommisionPerUser:
            sellerHasFreeCommission = true;
            break;
    }
}

if (!sellerHasFreeCommission)
{
    var sellerCommisionChargeTransaction = new Transaction
    {
        AccountId = sellerAccountId,
        Type = TransactionTypes.Fee,
        Amount = -2
    };

    var sellerTransferFeeTransaction = new Transaction
    {
        AccountId = 5020,
        Type = TransactionTypes.Fee,
        Amount = 2
    };

    _accountRepository.AddTransaction(sellerCommisionChargeTransaction);
    _accountRepository.AddTransaction(sellerTransferFeeTransaction);
}

if (!buyerHasFreeCommission)
{
    var buyerCommisionChargeTransaction  = new Transaction
    {
        AccountId = buyerAccountId,
        Type = TransactionTypes.Fee,
        Amount = -2
    };

    var buyerTransferFeeTransaction = new Transaction
    {
        AccountId = 5020,
        Type = TransactionTypes.Fee,
        Amount = 2
    };

    _accountRepository.AddTransaction(buyerCommisionChargeTransaction);
    _accountRepository.AddTransaction(buyerTransferFeeTransaction);
}

if (!buyerHasFreeCommission || !sellerHasFreeCommission)
    _accountRepository.SaveChanges();


This won't be very maintainable

Solution

The strategy pattern is a good candidate for this, one way you could implement it would be something like as follows (the discounts are the strategies).

Define an interface for classes which can apply discounts:

public interface IApplyDiscount
{
    void Apply(buyerUserId, buyerTransaction, sellerUserId, sellerTransaction);
}


Create an implementation of the interface for the SingleFreeCommisionPerUser discount.

public class SingleFreeCommisionPerUserDiscount : IApplyDiscount
{
    public void Apply(buyerUserId, buyerTransaction, sellerUserId, sellerTransaction)
    {
        var userDiscounts = _discountRepository.GetUserDiscountsForUsersWithDiscounts(new List { buyerUserId, sellerUserId }).ToList();

        var buyerHasFreeCommission = userDiscounts.Any(ud => ud.UserId == buyerUserId && ud.Discount.Type == DiscountTypes.SingleFreeCommisionPerUser);

        if(buyerHasFreeCommission)
        {
            buyerTransaction.Amount = 0;
        }

        var sellerHasFreeCommission = userDiscounts.Any(ud => ud.UserId == buyerUserId && ud.Discount.Type == DiscountTypes.SingleFreeCommisionPerUser);

        if(sellerHasFreeCommission)
        {
            sellerTransaction.Amount = 0;
        }
    }
}


The main code then creates the initial commission transactions:

var sellerCommisionChargeTransaction = new Transaction
{
    AccountId = sellerAccountId,
    Type = TransactionTypes.Fee,
    Amount = -2
};

var buyerCommisionChargeTransaction  = new Transaction
{
    AccountId = buyerAccountId,
    Type = TransactionTypes.Fee,
    Amount = -2
};


Applies all the available discounts:

List discounts = GetAllDiscounts();
discounts.ForEach(d => d.Apply(buyerUserId, buyerCommisionChargeTransaction, sellerUserId, sellerCommisionChargeTransaction));


Then, only add the corresponding charge if a transaction amount is due.

if (sellerCommisionChargeTransaction.Amount < 0)
{
    var sellerTransferFeeTransaction = new Transaction
    {
        AccountId = 5020,
        Type = TransactionTypes.Fee,
        Amount = -sellerCommisionChargeTransaction.Amount
    };

    _accountRepository.AddTransaction(sellerCommisionChargeTransaction);
    _accountRepository.AddTransaction(sellerTransferFeeTransaction);
}

if (buyerCommisionChargeTransaction.Amount < 0)
{
    var buyerTransferFeeTransaction = new Transaction
    {
        AccountId = 5020,
        Type = TransactionTypes.Fee,
        Amount = -buyerCommisionChargeTransaction.Amount
    };

    _accountRepository.AddTransaction(buyerCommisionChargeTransaction);
    _accountRepository.AddTransaction(buyerTransferFeeTransaction);
}


Then if you want to add other discount types and rates, you can just create additional IApplyDiscount implementations and include them in the results of the GetAllDiscounts() call.

This implementation is more flexible, you don't need to change any of the current implementation to add additional discounts.

The discounts could be percentages and the corresponding TransferFeeTransaction will cater for that by charging the positive value of whatever amount remains on the CommissionChargeTransaction.

Code Snippets

public interface IApplyDiscount
{
    void Apply(buyerUserId, buyerTransaction, sellerUserId, sellerTransaction);
}
public class SingleFreeCommisionPerUserDiscount : IApplyDiscount
{
    public void Apply(buyerUserId, buyerTransaction, sellerUserId, sellerTransaction)
    {
        var userDiscounts = _discountRepository.GetUserDiscountsForUsersWithDiscounts(new List<int> { buyerUserId, sellerUserId }).ToList();

        var buyerHasFreeCommission = userDiscounts.Any(ud => ud.UserId == buyerUserId && ud.Discount.Type == DiscountTypes.SingleFreeCommisionPerUser);

        if(buyerHasFreeCommission)
        {
            buyerTransaction.Amount = 0;
        }

        var sellerHasFreeCommission = userDiscounts.Any(ud => ud.UserId == buyerUserId && ud.Discount.Type == DiscountTypes.SingleFreeCommisionPerUser);

        if(sellerHasFreeCommission)
        {
            sellerTransaction.Amount = 0;
        }
    }
}
var sellerCommisionChargeTransaction = new Transaction
{
    AccountId = sellerAccountId,
    Type = TransactionTypes.Fee,
    Amount = -2
};

var buyerCommisionChargeTransaction  = new Transaction
{
    AccountId = buyerAccountId,
    Type = TransactionTypes.Fee,
    Amount = -2
};
List<IApplyDiscount> discounts = GetAllDiscounts();
discounts.ForEach(d => d.Apply(buyerUserId, buyerCommisionChargeTransaction, sellerUserId, sellerCommisionChargeTransaction));
if (sellerCommisionChargeTransaction.Amount < 0)
{
    var sellerTransferFeeTransaction = new Transaction
    {
        AccountId = 5020,
        Type = TransactionTypes.Fee,
        Amount = -sellerCommisionChargeTransaction.Amount
    };

    _accountRepository.AddTransaction(sellerCommisionChargeTransaction);
    _accountRepository.AddTransaction(sellerTransferFeeTransaction);
}

if (buyerCommisionChargeTransaction.Amount < 0)
{
    var buyerTransferFeeTransaction = new Transaction
    {
        AccountId = 5020,
        Type = TransactionTypes.Fee,
        Amount = -buyerCommisionChargeTransaction.Amount
    };

    _accountRepository.AddTransaction(buyerCommisionChargeTransaction);
    _accountRepository.AddTransaction(buyerTransferFeeTransaction);
}

Context

StackExchange Code Review Q#19672, answer score: 2

Revisions (0)

No revisions yet.