principlecsharpMinor
Does this ugly code fit into a design pattern?
Viewed 0 times
thisuglyintodesigndoescodefitpattern
Problem
I'm trying to figure out a pattern to best accommodate this pretty ugly code:
This won't be very maintainable
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:
Create an implementation of the interface for the
The main code then creates the initial commission transactions:
Applies all the available discounts:
Then, only add the corresponding charge if a transaction amount is due.
Then if you want to add other discount types and rates, you can just create additional
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
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.