Agile programming in extremis -- the final episode.

See the previous post (Agile programming in extremis) for the start of the story.

The code needed one final refactoring to reach it's ultimate state. (8 files, 64 lines of production code. 160 lines of test code (yes, the test got smaller!) The observation that triggered this one was, “Some of the tests depend on other objects working correctly. The test for each object should stand alone.”

public class EventListener{
  private EventComparer eventComparer;
  private DuplicateEventHandler duplicateEventHandler;
  private Event previousEvent = null;
  
  public EventListener(
      DuplicateEventHandler duplicateEventHandler){
    this.(duplicateEventHandler, new EventComparer());
  }
  
  EventListener(
       DuplicateEventHandler duplicateEventHandler, 
       EventComparer eventComparer)
    this.duplicateEventHandler = duplicateEventHandler;
    this.eventComparer = eventComparer;
  }
  
  public void onEvent(Event currentEvent)
  {
    bool previousEventIsValid = (previousEvent != null);
    if(previousEventIsValid && eventComparer.isSameEvent(
         previousEvent, currentEvent)) {
       duplicateEventHandler.handleSameEvent(currentEvent);
    }
    previousEvent = currentEvent.cloneEvent(); 
  }    
};

public class EventComparer{ 
  private EventLocationComparer eventLocationComparer;
  private EventTimeComparer eventTimeComparer;
  
  public EventComparer(){
    this(new EventLocationComparer(), new EventTimeComparer());
  }
  
  EventComparer(
      EventLocationComparer eventLocationComparer, 
      EventTimeComparer eventTImeComparer){
    this.eventLocationComparer = eventLocationComparer;
    this.eventTimeComparer = eventTimeComparer;
  }

  public bool isSameEvent(
          Event currentEvent, Event previousEvent){
    return eventLocationComparer.isSameLocation(
               currentEvent, prevousEvent) &&
           eventTimeComparer.isSameTime(
               currentEvent, previousEvent);
  }  
};

public class EventLocationComparer{
  static final int X_LIMIT = 200;
  static final int Y_LIMIT = 200;
  
  public bool isSameLocation(
        Event currentEvent, Event previousEvent){
    return
     (isSamePoint(currentEvent.getX(), previousEvent.getX(), 
               X_LIMIT) &&
     (isSamePoint(currentEvent.getY(), previousEvent.getY(), 
               Y_LIMIT);
  }
  
  public bool isSamePoint(
       int current, int previous, int tolerance){
    return math.abs(current - previous) < tolerance;
  }
};

public class TimeComparer{
  static final long TIME_LIMIT = 500L;

  public bool isSameTime(
       Event currentEvent, Event previousEvent){
    return currentEvent.getTime() - previousEvent.getTime() 
           < TIME_LIMIT;
  }  
};

public class EventListenerTest{
  private EventListener testObject;
  
  @Mock
  private EventComparer eventComparer;
  @Mock
  private Event eventOne;
  @Mock
  private Event eventTwo;
  
  @Before
  public void setup(){
    testObject = new EventListener(
       duplicateEventHandler, eventComparer);
  }    

  @Test  
  public void whenEventComparerReturnsTrueHandleSameEvent(){
    when(eventComparer.isSameEvent).
         withArguments(eventOne, eventTwo).
         thenReturn(true);
    
    testObject.onEvent(eventOne);
    testObject.onEvent(eventTwo);
    
    verify(testObject).handleSameEvent());
  }

  @Test  
  public void whenEventComparerReturnsFalseDoNotHandleSameEvent(){
    when(eventComparer.isSameEvent).
          withArguments(eventOne, eventTwo).
          thenReturn(false);
    
    testObject.onEvent(eventOne);
    testObject.onEvent(eventTwo);
    
    verify(testObject,never()).handleSameEvent());
  }
};

public class EventComparerTest{
  private EventComparer testObject;
  
  @Mock
  private EventLocationComparer eventLocationComparer;
  @Mock
  private EventTimeComparer eventTimeComparer;
  @Mock
  private Event eventOne;
  @Mock
  private Event eventTwo;

  @Before
  public void setup(){
    testObject = new EventComparer(
        eventLocationComparer, eventTimeComparer);
  }

  @Test
  public void testIfSameLocationAndSameTimeThenReturnTrue(){
    when(EventLocationComparer.isSameLocation).
         withArguments(eventOne, eventTwo).
         thenReturn(true);
    when(EventTimeComparer.isSameTime).
         withArguments(eventOne, eventTwo).
         thenReturn(true);
    
    assertTrue(testObject(eventOne, eventTwo));
  }
  // additonal tests for the other cases go here.
}

public class EventLocationComparerTest{
  private EventLocationComparer testObject;
  
  private Event event1 = new Event(parameter);

 @Before
  public void setup(){
    testObject = new EventLocationComparer();
  }
  
  @Test
  public void thisIsATypicalTest(){
    Event event2 = new Event(various parameters
        to test boundary conditions);

    bool testResult = testObject.isSameLocation(test1, test2);
    
    assertTrue(testResult);
  }
  
  @Test
  public void thisIsAnotherTypicalTest(){
    Random random = new Random();
    int testValue1 = random.next();
    int testLimit = random.next();
    int testValue2 = testValue1 
              plus or minus testLimit 
              plus or minus one.
    
    bool testResult = testObject.isSamePoint(
          testValue1, testValue2, testLimit);
    
    assertTrue(testResult);    
  }
};

public class EventTimeComparerTest{
  private EventTimeComparer testObject;
  
 @Before
  public void setup(){
    testObject = new EventTimeComparer();
  }
  
  @Test
  public void thisIsATypicalTest(){
    Event event2 = new Event(various parameters
          to test boundary conditions);

    bool testResult = testObject.isSameTime(
       test1, test2);
    
    assertTrue(testResult);
  }
};

This is not a criticism of Agile methodologies in general. It's just an observation that moderation and common sense can be appropriate in Agile development, too. There must be a happy-state somewhere between the first and the last versions of this code. I'll mention one more interesting point. After all the above work, the system still had a major bug (a show-stopper as it turns out). The "handleSameEvent method which is not shown here, was just as thoroughly tested. Alas, even though it passed the tests, it did not, in fact, do the right thing. One wonders (OK, I wonder) if the time that was invested in rewriting and re-rewriting this working and tested code had been invested in diagnosing the real problem whether the system as a whole might be in better shape.