I have a method called Buy
, when called it does the following:
- It calls a factory to create an order
- Calls pay on a payment object
- Calls empty on the customer's cart
- Calls the dao to save the order
This is what my sequence diagram says should happen, however, when I write my unit test for testing that it calls Empty
on the cart, it fails (null pointers) unless I specify that the factory returns an order.
So I end up faking behavior that isn't relevant, just to test if Empty
has been called on the cart.
Is this a recognised code smell? When you recognise this in your code, do you change it? I could obviously split up the empty cart functionality, but I do like the clean API of Buy
. Not to mention that splitting it up requires Empty
to be called first or last, which in this case isn't a problem, but in some situations might be.
This clutters my tests quite a bit, and it definitely smells like my method does more than it should, so any pointers to clean solutions would be appreciated.
//Implementation
public void Buy()
{
var order = CreateOrder();
Pay();
EmptyCart();
SaveChanges(order);
}
//Test
[TestMethod]
public void Buy_WhenCalled_CallsEmptyOnCart()
{
var cart = CreateFakeCartThatExpectsCallToEmpty();
var order = CreateFakeOrder().Object;
var orderFactory = CreateFakeOrderFactoryThatReturnsOrder(order).Object;
Cashier cashier = new Cashier(orderFactory, cart);
cashier.Buy();
cart.Verify();
}