1
\$\begingroup\$

Following on from this question: Using Specs when testing the process of assigning offers to a customer. I received some useful suggestions from t3chb0t and paparazzo. Therefore I have decided to revisit the code (without the specs). Here is a new version of the code:

public class Concor : IProduct
    {
        private const decimal _expenditure = 100;    
        public bool IsEligible(Customer customer)
        {
            return (customer.Expenditure < _expenditure);
        }
    }

    public class Chestnut : IProduct
    {
        private const decimal _expenditure = 100;

        public bool IsEligible(Customer customer)
        {
            return (customer.Expenditure >= _expenditure);
        }
    }

    public interface IProduct
    {
        bool IsEligible(Customer customer);
    }

    public class Customer
    {
        private readonly Guid _id;
        private readonly decimal _expenditure;
        private readonly IList<IProduct> _eligibleProducts = new List<IProduct>();
        public IEnumerable<IProduct> EligibleProducts
        {
            get { foreach (var product in _eligibleProducts) yield return product; }
        }

        public Guid Id
        {
            get
            {
                return _id;
            }
        }

        public Decimal Expenditure
        {
            get
            {
                return _expenditure;
            }
        }

        public Customer(Guid id, decimal expenditure)
        {
            _id = id;
            _expenditure = expenditure;
        }

        public void AddProduct(IProduct eligibleProduct)
        {
            _eligibleProducts.Add(eligibleProduct);
        }

        public IEnumerable<IProduct> DetermineEligibility(IList<IProduct> availableProducts)
        {
            return availableProducts.Where(x => x.IsEligible(this));
        }
    }

As it stands the client application will do something like this:

var eligibleProducts= customer.DetermineEligibility(availableProducts).ToList<IProduct>();
eligibleProducts.ToList().ForEach(c => customer.AddProduct(c));

Here are some points:

1) Customer now implements the IEnumerable method. I understand the reasoning for removing EligibilityCalculator, however I am wandering if Customer now has too many responsibilities.

2) I have corrected the issues with the _assignedProducts list from the last question: a) The private variable (_eligibleProducts) has an underscore. b) The public variable (EligibleProducts) does not have an underscore.

3) Renamed AddOffer to AddProduct.

4) Reduced the number of lines of code in: IsEligible to 1.

5) Removed the Gender constant.

6) Reduced the number of lines in DetermineEligibility (was CalculateEligibility) to 1.

7) Made private read only fields after introducing public getters.

I would be grateful for comments about the quality of the code and specifically the naming. Customers are pre approved for products, so I wander if eligible is the correct word.

My specific question surrounds the use of the DetermineEligibility method. Alternatively, I could change AddProduct to this (and remove DetermineEligibhility):

public void AddProducts(IList<IProduct> availableProducts)
        {
            var eligibleProducts = availableProducts.Where(x => x.IsEligible(this));
            eligibleProducts.ToList().ForEach(c => _eligibleProducts.Add(c));
        }
\$\endgroup\$

Your Answer

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

Browse other questions tagged or ask your own question.