Here's an example ..
class Payroll {
// ..
// ..
void calculateSalary(Employee emp) {
BigDecimal charge = emp.getSalary();
// calculate total salary including taxes
}
}
When I look at the possible unit test for the class
Payroll
, the first thing that strikes me is the argument Employee
in calculateSalary()
. The first attempt tries to instantiate an Employee
and invoke the method calculateSalary()
from the test case :class PayrollTest extends TestCase {
// ..
// ..
public void testSalaryCalculation() {
Employee emp = new Employee(..);
// ..
}
}
A Mockery of sorts ..
Instantiating an
Employee
object brings into action lots of other machineries like the database layer, the ORM layer etc., which we already know should be best kept away from the unit test harness. Think Mocks ..When you need to mock a concrete class, take a second look, if necessary take a third look as well. Mock roles, not objects and roles are best specified through interfaces. In the above example, we are really exploiting the fact that an employee is salaried and has a basic salary component for being employed in a company. In this method
calculateSalary()
, we are not concerned with the other details of the Employee
class, which may be used in other context within the software.Extract Interface
Now I have a new interface
Salaried
as :interface Salaried {
BigDecimal getBasicSalary();
}
and ..
public class Employee implements Salaried {
// ..
}
The method
calculateSalary()
now takes an interface Salaried
instead of the concrete class Employee
- and the mock becomes idiomatic in the test case (I am using EasyMock as the mocking framework) :class Payroll {
// ..
// ..
public void calculateSalary(Salaried emp) {
// ..
}
}
class PayrollTest extends TestCase {
// ..
// ..
public void testSalaryCalculation() {
IBillable mock = createMock(Salaried .class);
expect(mock.getBasicSalary()).andReturn(BigDecimal.valueOf(7000));
replay(mock);
// ..
}
}
Thus unit testing helped me use the Extract Interface principle and improve the collaboration between my classes. Notice how the class
Payroll
now collaborates with a more generic role Salaried
instead of a concrete class Employee
. Tomorrow if the company decides to employ contractors with different salary computation, we will have a separate abstraction :public class Contractor implements Salaried {
// ..
// ..
BigDecimal getBasicSalary() {
// .. different implementation than Employee
}
// ..
}
but the class
Payroll
remains unaffected, since it now collaborates with Salaried
and NOT any concrete implementation.
6 comments:
Nice post. We've recently written up why we avoid mocking concrete classes
But then you really didn't have to use EasyMock to determine that you could be using a Salaried interface. IMO a self review could have achieved the same result.
[for sabarish :] Very true, writing unit tests is one of the means of *self review* of the code that I have written. And using *mocks* is one way to streamline collaborations. And EasyMock is one of the most widely used mocking frameworks.
You might end up with a profusion of interfaces if you apply this principle all along. Some interfaces might be quite specific to single methods.
Is this a problem ? A valid concern ? Does it always go along with the data abstraction principle ?
[for pierre :]
If a concrete class plays multiple roles, then it is always recommended to publish the roles explicitly through interface driven modeling. In the example, one of the roles that the Employee abstraction was playing is that of a Salaried entity. And the method calculateSalary() of class Payroll is *only* concerned with the Salaried role of an Employee. Hence having the class Employee implement the Salaried interface is a valid "Extract Interface" refactoring. Initially I overlooked the possibility of having Salaried as a separate role of the entity. It was only during unit testing when I needed to mock a concrete class, did I realize that it could be a smell - and a refactoring was just around the corner.
I don't know. Maybe I would just start by passing a BigDecimal to calculateSalary. I will create the interface as soon as I feel the need for it (a second parameter is needed, another data type (than Employee) need to "expose" salary). It is difficult to say because the context of the example is not so detailed.
What do you think ?
Post a Comment