Monday, March 19, 2012

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.

Agile programming in extremis -- third refactoring's a charm.

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

The problem with the code from the previous post is "All those private methods are untestable." To resolve this we refactor again -- ending up with 8 files containing 50 lines of production code and approximately 180 lines of test code, like so:
public class EventListener{
  private EventComparer eventComparer = new EventComparer();
  private DuplicateEventHandler duplicateEventHandler;
  private Event previousEvent = null;
  
  public EventListener(
      DuplicateEventHandler duplicateEventHandler){
    this.duplicateEventHandler = duplicateEventHandler;
  }
  
  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 
        = new EventLocationComparer();
  private EventTimeComparer eventTimeComparer = 
           new EventTimeComparer();

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

public class LocationComparer{
  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;
  
  private Event event1 = new Event(parameter);

  @Mock
  private DuplicateEventHandler duplicateEventHandler;

  @Before  
  public void setup(){
    testObject = new EventListener(duplicateEventHandler);
    testObject.onEvent(event1);
  }    
  
  @Test
  public void thisIsATypicalTest(){
    Event event2 = new Event(various parameters
       to test boundary conditions);
    testObject.onEvent(event2);
    verify(duplicateEventHandler).handleSameEvent(event2);
    // or depending on the parameters to event2...
    verify(duplicateEventHandler, never()).
       handleSameEvent(event2);
  }
};

public class EventComparerTest{
  private EventComparer testObject;

  private Event event1 = new Event(parameter);

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

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

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);
  }
};
It may be a bit long, but hey, it's testable. (..or not. See the next post.)

Agile Programming in extremis -- re-refactoring.

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

The next person to look at the code said: "What does isSameEvent do? I can't tell from the code." The changes resulted in two files containing 39 lines of production code and 50 lines of test code.
public class EventListener{
  static final int X_LIMIT = 200;
  static final int Y_LIMIT = 200;
  static final long TIME_LIMIT = 500L;

  private DuplicateEventHandler duplicateEventHandler;
  private Event previousEvent = null;
  
  public EventListener(
      DuplicateEventHandler duplicateEventHandler){
    this.duplicateEventHandler = duplicateEventHandler;
  }
  
  public void onEvent(Event currentEvent)
  {
    bool previousEventIsValid = (previousEvent != null);
    if(previousEventIsValid 
         && isSameEvent(previousEvent, currentEvent)) {
       duplicateEventHandler.handleSameEvent(currentEvent);
    }
    previousEvent = currentEvent.cloneEvent(); 
  }    
  
  private bool isSameEvent(
      Event currentEvent, Event previousEvent){
    return isSameLocation(currentEvent, prevousEvent) &&
           isSameTime(currentEvent, previousEvent);
  }
  
  private bool isSameLocation(
        Event currentEvent, Event previousEvent){
    return (isSamePoint(
               currentEvent.getX(), 
               previousEvent.getX(), 
               X_LIMIT) &&
           (isSamePoint(
               currentEvent.getY(), 
               previousEvent.getY(), 
               Y_LIMIT);
  }
  
  private bool isSameTime(
       Event currentEvent, Event previousEvent){
    return currentEvent.getTime() - previousEvent.getTime() 
      < TIME_LIMIT;
  }
  
  private bool isSamePoint(
       int current, int previous, int tolerance){
    return math.abs(current - previous) < tolerance;
  }
};

public class EventListenerTest{
  private EventListener testObject;
  
  private Event event1 = new Event(parameter);

  @Mock
  private DuplicateEventHandler duplicateEventHandler;

  @Before  
  public void setup(){
    testObject = new EventListener(
         duplicateEventHandler);
    testObject.onEvent(event1);
  }    
  
  @Test
  public void thisIsATypicalTest(){
    Event event2 = new Event(varying parameters 
       to test boundary conditions);
    testObject.onEvent(event2);
    verify(duplicateEventHandler).handleSameEvent(event2);
    // or depending on the parameters to event2...
    verify(duplicateEventHandler, never()).
         handleSameEvent(event2);
  }
};

Agile programming in extremis -- the first refactoring.

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

The first agile developer to examine this code said: “It’s not clear what that if statement is testing.” and produced the following improved version (2 files; 28 lines of production code; around 50 lines of test code):
public class EventListener{
  static final int X_LIMIT = 200;
  static final int Y_LIMIT = 200;
  static final long TIME_LIMIT = 500L;

  private DuplicateEventHandler duplicateEventHandler;
  private Event previousEvent = null;
  
  public EventListener(
       DuplicateEventHandler duplicateEventHandler){
    this.duplicateEventHandler = duplicateEventHandler;
  }
  
  public void onEvent(Event currentEvent)
  {
    bool previousEventIsValid = (previousEvent != null);
    if(previousEventIsValid && 
         isSameEvent(previousEvent, currentEvent)) {
       duplicateEventHandler.handleSameEvent(currentEvent);
    }
    previousEvent = currentEvent.cloneEvent(); 
  }    
  
  private bool isSameEvent(
       Event currentEvent, Event previousEvent){
    return 
      ((math.abs(currentEvent.getX() – previousEvent.getX()) 
         < X_LIMIT) &&
       (math.abs(currentEvent.getY() – previousEvent.getY()) 
         < Y_LIMIT) &&
       (currentEvent.getTime – previousEvent.getTime() 
         < TIME_LIMIT));
  }
};

public class EventListenerTest{
  private EventListener testObject;
  
  private Event event1 = new Event(parameter);

  @Mock
  private DuplicateEventHandler duplicateEventHandler;

  @Before  
  public void setup(){
    testObject = new EventListener(duplicateEventHandler);
    testObject.onEvent(event1);
  }    
  
  @Test
  public void thisIsATypicalTest(){
    Event event2 = new Event(various parameters
       to test boundary conditions);
    testObject.onEvent(event2);
    verify(duplicateEventHandler).handleSameEvent(event2);
    // or depending on the parameters to event2...
    verify(duplicateEventHandler, never()).handleSameEvent
         (event2);
  }
};

Agile programming in extremis

I was recently working on a project with a team that had adopted Agile development methods with a vengeance.  In the next few posts, I'll report on the evolution of some code during repeated refactorings.

To protect the innocent and obscure the guilty, I am paraphrasing the actual code and omitted some of the boilerplate and other detail, but rest assured that Dave Barry would agree that "I am not making this up."

To get started, here is the original code.  There are two files. Before I formatted it for this blog it containing 23 lines of production code and around 50 lines of test code (of which 23 are shown here.)

public class EventListener{
  static final int X_LIMIT = 200;
  static final int Y_LIMIT = 200;
  static final long TIME_LIMIT = 500L;

  private final DuplicateEventHandler duplicateEventHandler;
  private Event previousEvent = null;
  
  public EventListener(
         DuplicateEventHandler duplicateEventHandler){
    this.duplicateEventHandler = duplicateEventHandler;
  }
  
  public void onEvent(Event currentEvent)
  {
    if( previousEvent != null &&
       (math.abs(currentEvent.getX() – previousEvent.getX()) 
          < X_LIMIT) &&
       (math.abs(currentEvent.getY() – previousEvent.getY()) 
          < Y_LIMIT) &&
       (currentEvent.getTime – previousEvent.getTime() 
          < TIME_LIMIT)) {
       duplicateEventHandler.handleSameEvent(currentEvent);
    }
    previousEvent = currentEvent.cloneEvent(); 
  }
};

public class EventListenerTest{
  private EventListener testObject;
  
  private Event event1 = new Event(parameter);

  @Mock
  private DuplicateEventHandler duplicateEventHandler;

  @Before  
  public void setup(){
    testObject = new EventListener(duplicateEventHandler);
    testObject.onEvent(event1);
  }    
  
  @Test
  public void thisIsATypicalTest(){
    Event event2 = new Event(various parameters 
           to test boundary conditions);
    testObject.onEvent(event2);
    verify(duplicateEventHandler).handleSameEvent(event2);
    // or depending on the parameters to event2...
    verify(duplicateEventHandler, never()).handleSameEvent(
       event2);
  }
};
If you are a programmer, now would be a good time to examine the code above and decide what changes, if any, you would make to improve it.

Three and a bit years lapse....

I haven't added anything to this blog since January 2009.  Lots has happened, but apparently a facebook status update is sufficient.   Now I have something more meaty to talk about, so....

Wednesday, January 28, 2009

Tardis

Someday my Tardis is going to have been on backorder by now, but apparently some people have their's already.

I say this because I have a new netbook on order.

I was an early adopter on this one. It went on sale the 20th and I ordered that morning.

The original scheduled ship date was 1/27 (yesterday as I write this) but instead I got an email that said:

Dear WILSON DALE

Due to a delay in fulfilling your order # xxxxx placed on 1/20/2009, it may miss the estimated ship date that was on your original order confirmation. We sincerely apologize for this inconvenience.

However there are already five "customer reviews" of this computer out on the HP web site, so either these folks have their Tardis up and running or they are reviewing a computer that they haven't even seen yet.

For what it's worth: 3 of 5 (60%) customers recommend this product.

[Oh, alright, maybe they got advanced copies for review, or are basing their review on what they saw at CES, but I doubt it, and in either case that doesn't qualify as a "customer review" in my book.]

Sunday, August 10, 2008

My Contra Dance

I've been doing a lot of contra dancing recently, and have tried my hand at writing some dances. I've also been experimenting with Google Docs as a way to publish them.
For example:

Fly Around My Pretty Little Miss

Saturday, June 21, 2008

Contra Dancing

As I mention in my profile, I have a new obsession -- contra dancing with occasional forays into English Country Dancing. Obsessions come; obsessions go; sometimes they stick around. I'm hoping this one will stick. It s great fun, great exercise, and I've met some really wonderful people at dances.

Any good obsession should fit in with my previous obsessions. The connection to folk music and upright bass is obvious. Computers are a bit more of a stretch, but in my (ahem) spare time, I have started working on a program to help design dances and/or catalog existing dances. Then there's the weaving.

A contra dance is all about patterns fitting in a fairly constrained framework. Weaver's will feel right at home. The question is can there be cross-over. Could one weave a contra dance, or dance a weave structure. Hmmmm.....

If you're in the St. Louis area check the Childgrove site linked above, and come to a dance (usually Sunday night.) You'll be glad you did.

Oh, and the link to parrots? Well, Willow went to a flash dance recently. She was a big hit.

Wednesday, January 16, 2008

Artificial Inscrutability vs Human Pattern Recognition: A Challenge

I was messing with the automatic language translation sites on the Internet. I started with the lyrics of a song, translated them from English to French to German, back to French, then back to English, and came up with:

The scandal is currently sufficient
May with the sun them on you
that supplements expensive you;
And the pure light with you
to lead your house in the kind.


Anyone recognize the original song?

Hints:

  • The song came from an album released in 1968 so there's an age bias in this challenge.
  • The album has been reissued on CD -- I was pleased.
  • I believe the lyrics were actually borrowed from a traditional folk song.
  • Somewhere along the line the translation acquired an extra line (and some extra concepts!) The original verse was four lines long.
  • The original lyrics were comprehensible, although I've got to admit you could probably substitute the translated version into the song and few people would notice.

Thursday, November 29, 2007

Simplicity

My previous rant was in response the frustration that built up while I was reading various standards documents and also using some ACE code [1]. In the interests of simplicity, however I should have just quoted Stephen Dewhurst from his book C++ Common Knowledge:

It's surprising how much of advanced object-oriented programming is basic common sense surrounded by impenetrable syntax.


Dale


[1] Pop quiz: An ACE_Map_Manager does not "manage" maps, it is a map. A map is a useful container for data. There are a couple of ways to describe the function it provides:

As a map: Given a key, map that into a corresponding value.
As a dictionary: Given a word, provide a definition.
As an associative array: Given an index of arbitrary type, return the associated value.

These points of view are equivalent (although the dictionary view implies that keys and values are text which is too restrictive). They describe useful ways to think about ACE_Map as a tool for solving a real programming problems. In each case there are two interesting values: The key (or word, or index), and the associated value (or definition).

In ACE, these are called int_id and ext_id (not necessarily in that order).

Question: Which one is the key and which one is the value?

Hint: int stands for internal (not the C++ data type int); and ext stands for external.

So int_id must be the data that is stored inside the map and ext_id is name the external world uses to get to that data.

No, now that I think of it int_id is the name used internally to identify the data and ext_id is the data that is of interest to the external world.

Hmmmm..

Answer: I have no idea. I have to look it up (in the code because the documentation doesn't help!) every time I use an ACE_Map or else find some existing code that uses it and copy/paste it into the code I am writing.

Extra Credit Question: ACE has been around well over ten years now and is used in thousands of applications. How many programmer-years have been wasted trying to remember which "id" is which?

Monday, November 26, 2007

'Tis a gift to be simple

This used to be a long rambling blog entry in which I complained about the tendency of technical folks, to oversimplify complex problems, then add the complexity back in by using obscure terminology accessible only to the in-crowd.

The next entry, which in the topsy-turvy world of blogs is up ^^^ there, and which you've probably already read, quoted Stephen Dewherst who said it much more clearly and concisely.

So I rewrote this entry.

Hey if I can't change history in my own blog, where can I change it?