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.
