16 October 2006

When to ask instead of tell?

Mock objects work best with, and are designed to support, object-oriented code that is written in a Tell, Don't Ask style. However, that doesn't mean objects must, or even can, never ask each other for information. But apart from value objects and generic collection types, neither of which are stateful domain objects, I've found that I only need queries to search, sort and filter collections of objects. For example to filter non-managers out of a list of employees I'd need an isManager method on the Employee class:
Set managers() {
    Set managers = new HashSet();
    for (Employee employee : employees) {
        if (employee.isManager()) {
            managers.add(employee);
        }
    }
    return managers;
}
To use mock objects to test a method that only applies to managers you would stub calls to the isManager method of employees in the collection:
public void testOnlyAllowsManagersToTravelBusinessClass() { // for example
    Employee peon = mock(Employee.class, "peon");
    Employee manager = mock(Employee.class, "manager");
    
    Company company = new Company();
    company.addEmployees(peon, manager);
    
    expects(new InAnyOrder() {{
        allowing (peon).isManager(); will(returnValue(false));
        allowing (manager).isManager(); will(returnValue(true));
        
        ...
    }});
    
    ...
}
Queries should not require train-wreck expressions because that couples the object doing the selecting or sorting to the structure of the objects in the collection. The following code is a bit dodgy. The train-wreck expression will make code difficult to test with mock objects because a test will need to create a mock for each link in the chained expression.
Person findAManager(List employees) {
    List managers = new List();
    for (Employee employee : employees) {
        if (employee.getJobDescription().getRoles().contains(Role.MANAGER)) {
            managers.add(employee);
        }
    }
    return managers;
}
Introducing an isManager method on the Employee class makes testing easier and express the intent of the code more clearly.

2 comments:

Anonymous said...

Nat-

I like your suggestions on getting rid of "train-wreck" code, and look forward to thinking about how to get rid of that anti-pattern in my own code.

But I wanted to ask about your mocking of Employee. Is that something you would really do, or is it just a contrived example? For code like this, I would normally just create manager and non-manager test objects and use them in the test. Mocking requires you to extract an employee interface (or use cglib). I would normally write a test for this method more like this:

public void testOnlyAllowsManagersToTravelBusinessClass() { // for example
Employee peon = createNonManager();
Employee manager = createManager();

Company company = new Company();
company.addEmployees(peon, manager);
...
}});

...
}

// convenience method in test class
public Employee createManager() {
Employee manager = new Employee();
manager.promoteToManager();
return manager;
}

...

Nat Pryce said...

It's a contrived example!

In real code I'd prefer to have role-based interfaces that objects can implement. If you want to select objects that represent positions in a corporate hierarchy, perhaps to promote one, you might give objects a Promotable interface.