jeudi 16 juillet 2015

How to avoid class self-use

I have the following class:

public class MyClass
{

    public void deleteOrganization(Organization organization)
    {
        /*Delete organization*/

        /*Delete related users*/
        for (User user : organization.getUsers()) {
            deleteUser(user);
        }
    }

    public void deleteUser(User user)
    {
        /*Delete user logic*/
    }
}

This class represents a self-use in that its public method deleteOrganization uses its other public method deleteUser. In my case this class is a legacy code against which I started adding unit tests. So I started by adding a unit test against the first method deleteOrganization and ended up figuring out that this test has been extended to also test the deleteUser method.

Problem

The problem is that this test is not isolated anymore (it should only test the deleteOrganization method). I was obliged to treat the different conditions related to deleteUser method in order to pass it so to pass the test, which has hugely increased the complexity of the test.

Solution

The solution was to spy the class under test and stub deleteUser method:

@Test
public void shouldDeleteOrganization()
{
    MyClass spy = spy(new MyClass());

    // avoid invoking the method
    doNothing().when(spy).deleteUser(any(User.class));

    // invoke method under test
    spy.deleteOrganization(new Organization());
}

New Problem

Though the previous solution solves the problem, it's not recommended as the javadoc of the spy method states:

As usual you are going to read the partial mock warning: Object oriented programming is more less tackling complexity by dividing the complexity into separate, specific, SRPy objects. How does partial mock fit into this paradigm? Well, it just doesn't... Partial mock usually means that the complexity has been moved to a different method on the same object. In most cases, this is not the way you want to design your application.

The complexity of the deleteOrganization method has been moved to deleteUser method and this is caused by the class self-use. The fact that this solution is not recommended, in addition to the In most cases, this is not the way you want to design your application statement, suggests that there is a code smell, and refactoring is indeed needed to improve this code.

How to remove this self-use? Is there a design pattern or a refactoring technique that can be applied?

Aucun commentaire:

Enregistrer un commentaire