In Need of Refactoring in Order to Improve Testability

Question

Hi, I'm testing a simple DAO layer with mockito but I found a problem, basically a hard to test interface and I was wondering if you could give me some insight...

This is the method I want to test:

public Person getById(UserId id) {
    final Person person = new PersonImpl();

    gateway.executeQuery(GET_SQL + id.getUserId(), new ResultSetCommand(){
      public int work(ResultSet rs) throws SQLException {
        if(rs.next()){
          person.getName().setGivenName(rs.getString("name"));
          person.getName().setFamilyName(rs.getString("last_name"));
        }
        return 0;
      }
    });
    return person;
  }

I use a DatabaseGateway that is my interface between java code and SQL, and that method accepts an anonymous class, this is the method executeQuery of the gateway:

 public int executeQuery(String sql, ResultSetCommand cmd) {
    try{
      Connection cn =  createConnection();
      PreparedStatement st = cn.prepareStatement(sql);
      int result = cmd.work(st.executeQuery());
      cn.close();
      return result;
    }catch(Exception e){
      throw new RuntimeException("Cannot Create Statement for sql " + sql,e);
    }
  }

The thing is that, because of that anonymous class, It's getting harder to test PersonDAO.

I can refactor the whole code, even remove the anonymous class if someone suggest a better design (I'm sure there's a simpler one, but I just can't seem to find it).

Thanks everyone for the suggestions.

PD: if you need further information, feel free to ask

---

EDIT: Test that's hard to do

public void testGetPersonById(){
    DatabaseGateway gateway = mock(DatabaseGateway.class);
    when(gateway.executeQuery(anyString(),any(ResultSetCommand.class)));
    PersonDAO person_dao = new PersonDAOImpl(gateway);

    Person p = person_dao.getById(new UserId(Type.viewer,"100"));
  }

See? ResultCommand is part of the mock, and I'm interested in testing that code too... should I do a separate test for that specific command?

Answer 1

Instead of using anonymous classes, you could create an interface and its implementation separately. The method executeQuery would then have a String and this interface as a parameter.

So your test would remain the same. You would be able to separate the work method in another test (the test for your interface implementation), which looks like is the hard thing to test.

The result would be something like:

public Person getById(UserId id) {
    final Person person = new PersonImpl();

    gateway.executeQuery(GET_SQL + id.getUserId(), new MyInterfaceImpl(person));
    return person;
}

,

public int executeQuery(String sql, MyInterface cmd) {
    try{
      Connection cn =  createConnection();
      PreparedStatement st = cn.prepareStatement(sql);
      int result = cmd.work(st.executeQuery());
      cn.close();
      return result;
    }catch(Exception e){
      throw new RuntimeException("Cannot Create Statement for sql " + sql,e);
    }
  }

Answer 2

You could get fancy and "capture" the ResultSetCommand arg, then simulate the callback on it with a mock ResultSet:

/**
 * Custom matcher - always returns true, but captures the
 * ResultSetCommand param
 */
class CaptureArg extends ArgumentMatcher<ResultSetCommand> {
    ResultSetCommand resultSetCommand;
    public boolean matches(Object resultSetCommand) {
         resultSetCommand = resultSetCommand;
         return true;
    }
}

public void testGetPersonById(){
    // setup expectations...
    final String lastName = "Smith";
    final String firstName = "John";
    final CaptureArg captureArg = new CaptureArg();
    DatabaseGateway gateway = mock(DatabaseGateway.class);
    ResultSet mockResultSet = mock(ResultSet.class);
    when(gateway.executeQuery(anyString(), argThat(captureArg) ));
    when(mockResultSet.next()).thenReturn(Boolean.True);
    when(mockResultSet.getString("name")).thenReturn(firstName);
    when(mockResultSet.getString("last_name")).thenReturn(lastName);

    // run the test...
    PersonDAO person_dao = new PersonDAOImpl(gateway);
    Person p = person_dao.getById(new UserId(Type.viewer,"100"));

    // simulate the callback...
    captureArg.resultSetCommand.work(mockResultSet);

    // verify
    assertEquals(firstName, person.getName().getGivenName());
    assertEquals(lastName, person.getName().getFamilyName());
}

I'm conflicted as to whether I like this - it exposes a lot of the internals of the method you're testing. But at least it's an option.