How to unit test a function that is refactored to strategy pattern?

If I have a function in my code that goes like:

class Employee{

    public string calculateTax(string name, int salary)
    {
        switch (name)
        {
            case "Chris":
                doSomething($salary);
            case "David":
                doSomethingDifferent($salary);
            case "Scott":
               doOtherThing($salary);               
       }
}

Normally I would refactor this to use Ploymorphism using a factory class and strategy pattern:

public string calculateTax(string name)
{
    InameHandler nameHandler = NameHandlerFactory::getHandler(name);
    nameHandler->calculateTax($salary);
}

Now if I were using TDD then I would have some tests that work on the original calculateTax() before refactoring.

ex:

calculateTax_givenChrisSalaryBelowThreshold_Expect111(){}    
calculateTax_givenChrisSalaryAboveThreshold_Expect111(){}

calculateTax_givenDavidSalaryBelowThreshold_Expect222(){}   
calculateTax_givenDavidSalaryAboveThreshold_Expect222(){} 

calculateTax_givenScottSalaryBelowThreshold_Expect333(){}
calculateTax_givenScottSalaryAboveThreshold_Expect333(){}

After refactoring I’ll have a Factory class NameHandlerFactory and at least 3 implementation of InameHandler.

How should I proceed to refactor my tests? Should I delete the unit test for claculateTax() from EmployeeTests and create a Test class for each implementation of InameHandler?

Should I test the Factory class too?

The old tests are just fine for verifying that calculateTax still works as it should. However, you don’t need many test cases for this, only 3 (or maybe some more, if you want to test error handling too, using unexpected values of name).

Each of the individual cases (at the moment implemented in doSomething et al.) must have their own set of tests too, which test the inner details and special cases related to each implementation. In the new setup these tests could / should be converted into direct tests on the respective Strategy class.

I prefer to remove old unit tests only if the code they exercise, and the functionality it implements, completely ceases to exist. Otherwise, the knowledge encoded into these tests is still relevant, only the tests need to be refactored themselves.

Update

There may be some duplication between the tests of calculateTax (let’s call them high level tests) and the tests for the individual calculation strategies (low level tests) – it depends on your implementation.

I guess the original implementation of your tests asserts the result of the specific tax calculation, implicitly verifying that the specific calculation strategy was used to produce it. If you keep this schema, you will have duplication indeed. However, as @Kristof hinted, you may implement the high level tests using mocks too, to verify only that the right kind of (mock) strategy was selected and invoked by calculateTax. In this case there will be no duplication between high and low level tests.

So if refactoring the affected tests isn’t too costly, I would prefer the latter approach. However, in real life, when doing some massive refactoring, I do tolerate a small amount of test code duplication if it saves me enough time 🙂

Should I test the Factory class too?

Again, it depends. Note that the tests of calculateTax effectively test the factory. So if the factory code is a trivial switch block like your code above, these tests may be all you need. But if the factory does some more tricky things, you may want to dedicate some tests specifically for it. It all boils down to how much tests you need to be confident that the code in question really works. If, upon reading the code – or analysing code coverage data – you see untested execution paths, dedicate some more tests to exercise these. Then repeat this until you are fully confident in your code.

2

I’ll start by saying that I’m no expert on TDD or unit testing, but here’s how I would test this (I’ll use pseudo-like code):

CalculateTaxDelegatesToNameHandler()
{
    INameHandlerFactory fakeNameHandlerFactory = Fake(INameHandlerFactory);
    INameHandler fakeNameHandler = Fake(INameHandler);

    A.Call.To(fakeNameHandlerFactory.getHandler("John")).Returns(fakeNameHandler);

    Employee employee = new Employee(fakeNameHandlerFactory);
    employee.CalculateTax("John");

    Assert.That.WasCalled(fakeNameHandler.calculateTax());
}

So I’d test that the calculateTax() method of employee class correctly asks its NameHandlerFactory for a NameHandler and then calls the calculateTax() method of the returned NameHandler.

2

You’re taking one class (employee that does everything) and making 3 groups of classes: the factory, the employee (that just contains a strategy) and the strategies.

So make 3 groups of tests:

  1. Test the factory in isolation. Does it handle inputs correctly. What happens when you pass in an unknown?
  2. Test the employee in isolation. Can you set an arbitrary strategy and it works as you expect? What happens if there is no strategy or factory set? (if that’s possible in code)
  3. Test the strategies in isolation. Does each perform the strategy you expect? Do they handle odd boundary inputs in a consistent manner?

You can of course make automated tests for the whole shebang, but these are now more like integration tests and should be treated as such.

Before writing any code, I would start with a test for a Factory. Mocking the stuff I need I would force myself to think about the implementations and usecases.

Than I would implement a Factory and continue with a test for each implementation and finally the implementations themselves for those tests.

Finally I would remove the old tests.

My opinion is that you should do nothing, meaning you should not add any new tests.

I stress that this is an opinion, and it actually depends on the way you perceive the expectations from the object. Do you think the user of the class would like to supply a strategy for tax calculation? If he doesn’t care, then the tests should reflect that, and the behaviour reflected from the unit tests should be that they should not care that the class has started to use a strategy object to calculate tax.

I actually ran into this problem several times when using TDD. I think the main reason is that a strategy object is not a natural dependency, as opposed to say an architectural boundary dependency like an external resource (a file, a DB, a remote service, etc.). Since it is not a natural dependency, I usually don’t base the behaviour of my class on this strategy. My instinct is that I should only change my tests if the expectations from my class have changed.

There’s a great post from Uncle Bob, that talks exactly about this problem when using TDD.

I think that the tendency to test each separate class is what’s killing TDD. The whole beauty of TDD is that you use tests to spur up design schemes and not vice versa.

Trang chủ Giới thiệu Sinh nhật bé trai Sinh nhật bé gái Tổ chức sự kiện Biểu diễn giải trí Dịch vụ khác Trang trí tiệc cưới Tổ chức khai trương Tư vấn dịch vụ Thư viện ảnh Tin tức - sự kiện Liên hệ Chú hề sinh nhật Trang trí YEAR END PARTY công ty Trang trí tất niên cuối năm Trang trí tất niên xu hướng mới nhất Trang trí sinh nhật bé trai Hải Đăng Trang trí sinh nhật bé Khánh Vân Trang trí sinh nhật Bích Ngân Trang trí sinh nhật bé Thanh Trang Thuê ông già Noel phát quà Biểu diễn xiếc khỉ Xiếc quay đĩa Dịch vụ tổ chức sự kiện 5 sao Thông tin về chúng tôi Dịch vụ sinh nhật bé trai Dịch vụ sinh nhật bé gái Sự kiện trọn gói Các tiết mục giải trí Dịch vụ bổ trợ Tiệc cưới sang trọng Dịch vụ khai trương Tư vấn tổ chức sự kiện Hình ảnh sự kiện Cập nhật tin tức Liên hệ ngay Thuê chú hề chuyên nghiệp Tiệc tất niên cho công ty Trang trí tiệc cuối năm Tiệc tất niên độc đáo Sinh nhật bé Hải Đăng Sinh nhật đáng yêu bé Khánh Vân Sinh nhật sang trọng Bích Ngân Tiệc sinh nhật bé Thanh Trang Dịch vụ ông già Noel Xiếc thú vui nhộn Biểu diễn xiếc quay đĩa Dịch vụ tổ chức tiệc uy tín Khám phá dịch vụ của chúng tôi Tiệc sinh nhật cho bé trai Trang trí tiệc cho bé gái Gói sự kiện chuyên nghiệp Chương trình giải trí hấp dẫn Dịch vụ hỗ trợ sự kiện Trang trí tiệc cưới đẹp Khởi đầu thành công với khai trương Chuyên gia tư vấn sự kiện Xem ảnh các sự kiện đẹp Tin mới về sự kiện Kết nối với đội ngũ chuyên gia Chú hề vui nhộn cho tiệc sinh nhật Ý tưởng tiệc cuối năm Tất niên độc đáo Trang trí tiệc hiện đại Tổ chức sinh nhật cho Hải Đăng Sinh nhật độc quyền Khánh Vân Phong cách tiệc Bích Ngân Trang trí tiệc bé Thanh Trang Thuê dịch vụ ông già Noel chuyên nghiệp Xem xiếc khỉ đặc sắc Xiếc quay đĩa thú vị
Trang chủ Giới thiệu Sinh nhật bé trai Sinh nhật bé gái Tổ chức sự kiện Biểu diễn giải trí Dịch vụ khác Trang trí tiệc cưới Tổ chức khai trương Tư vấn dịch vụ Thư viện ảnh Tin tức - sự kiện Liên hệ Chú hề sinh nhật Trang trí YEAR END PARTY công ty Trang trí tất niên cuối năm Trang trí tất niên xu hướng mới nhất Trang trí sinh nhật bé trai Hải Đăng Trang trí sinh nhật bé Khánh Vân Trang trí sinh nhật Bích Ngân Trang trí sinh nhật bé Thanh Trang Thuê ông già Noel phát quà Biểu diễn xiếc khỉ Xiếc quay đĩa
Thiết kế website Thiết kế website Thiết kế website Cách kháng tài khoản quảng cáo Mua bán Fanpage Facebook Dịch vụ SEO Tổ chức sinh nhật