trjhtr

I asked this question earlier: Testing the process of assigning offers to a customer. Following on from the answerers comprehensive response. I have attempted it again. Please see the test code below:

[TestFixture]
public class CustomerTest

 private IList<IProduct> _products;
 private Concor _concor;
 private Chestnut _chestnut;

 private IOfferCalculator _offerCalculator;

 [OneTimeSetUp]
 public void OneTimeSetUp()
 
 _offerCalculator = new OfferCalculator();
 

 [SetUp]
 public void Setup()
 
 _concor = new Concor();
 _chestnut = new Chestnut();
 _products = new List<IProduct>();
 _products.Add(_concor);
 _products.Add(_chestnut);
 

 [Test]
 public void Should_receive_a_for_concor()
 
 var expectedConcor = new List<IProduct>();
 expectedConcor.Add(_concor);
 var gender = "F";
 var expenditure = 1500M;
 var customer = new Customer(gender,expenditure);
 //Act
 var eligibleProducts = customer.GetEligibleOffers(_offerCalculator, _products);
 customer.AssignOffers(eligibleProducts.ToList());
 //Assert
 CollectionAssert.AreEqual(customer._assignedProducts, expectedConcor);
 

Here is the supporting code:

public class Concor : IProduct
 
 //Added constants
 private const decimal _expenditure =100;
 private const string _gender = "F";

 public bool IsEligible(string gender, decimal expenditure)
 
 //Added constants. Not sure whether they should be private or public. Surely they should be private and not public just so users
 //of the Domsain Model can see them?
 if (expenditure > _expenditure && gender == _gender)
 
 return true;
 
 else
 
 return false;
 
 
 

 public class Chestnut : IProduct
 
 //Added constants. Not sure whether they should be private or public. Surely they should be private and not public just so users
 //of the Domsain Model can see them?
 private const decimal _expenditure = 100;
 private const string _gender = "M";

 public bool IsEligible(string gender, decimal expenditure)
 
 //Remove magic string and magic decimal
 if (expenditure > _expenditure && gender == _gender)
 
 return true;
 
 else
 
 return false;
 
 
 

 //Replaced abstract class with interface
 public interface IProduct
 
 //This method now accepts parameters rather than an object
 bool IsEligible(string gender, decimal expenditure);
 

 public interface IOfferCalculator
 
 //Renamed this method.
 IEnumerable<IProduct> CalculateEligibility(string gender, decimal expenditure, IList<IProduct> products);
 

 public class Customer
 
 public string Gender get; protected set; 
 public decimal Expenditure get; protected set; 
 public IList<IProduct> _assignedProducts = new List<IProduct>();

 public Customer(string gender, decimal expenditure)
 
 Gender = gender;
 Expenditure = expenditure;
 

 //Added this method as a result of refactoring AssignOffers.
 public IEnumerable<IProduct> GetEligibleOffers(IOfferCalculator offerCalculator, IList<IProduct> products)
 
 //Pass Gender and Expenditure instead of this object (Customer).
 return offerCalculator.CalculateEligibility(Gender, Expenditure, products);
 

 //Refactored this method. 
 public void AssignOffers(IList<IProduct> products)
 
 foreach (var product in products)
 
 _assignedProducts.Add(product);
 
 
 

 public class OfferCalculator : IOfferCalculator
 
 public IEnumerable<IProduct> CalculateEligibility(string gender, decimal expenditure, IList<IProduct> products)
 
 foreach (var product in products)
 
 if (product.IsEligible(gender,expenditure))
 
 yield return product;
 
 
 
 

I have included comments to show what I have changed.

I would like some feedback about the quality of the test. The company like to use BDD, however I am conscious that I asserting state rather than behaviour (arguing that I am testing calculations rather than orchestrations). I would also like some feedback about my use of the: OneTimeSetup attribute and Setup attribute. Also How would I test for a Chestnut? Add a new method or use an attribute for the test cases?