Legacy pictures

July 18 2010

someone once stated that legacy code is simply “code that works” (I guess was Michael Feathers, but could not find any reference online). Feathers himself proposed characterization tests as a vise to put legacy code into, before starting doing any change.

this week had the pleasure to add support to our current application for customer’s internal search engine indexing, something already developed by some colleagues of mine, for a similar web application. so, I spent some time reading product documentation and reviewing existing code, which was clearly composed of custom domain logic and generic integration logic with given product. I also played a bit with code, in a sandbox, to understand how pieces could be splitted.

then, I reverted every change, and started writing an initial characterization test.

main application entry point was a “workflow” process, expected to be executed by the application container when a few events had occurred. visible behaviour consisted of text files, with specific content, stored via FTP. so, I first “reproduced” current behaviour starting a local FTP server (bundled with my MacBook, and available from System Preferences). after having seen text files written, I could write an automatic test (well, automatic but not yet reproducible on every workstation).

@Before
public void setUp() throws Exception {
  ...

  ftpFolder = createTmpFolderAt(FTP_HOME_FOLDER);

  properties.put("ftp.url", FTP_HOST);
  properties.put("ftp.port", FTP_PORT);
  properties.put("ftp.user", FTP_USER);
  properties.put("ftp.password", FTP_PASSWORD);
  properties.put("ftp.directory", ftpFolder.getName());
  properties.put("base.url", "http://www.mysite.com");
  
  Hashtable<String, String> properties = new Hashtable<String, String>();
  activate(facade, new InMemoryComponentContext(properties));
}

@Test
public void shouldNotifyNewsCreation() throws Exception {
  String lavoro = "/its/a/news/lavoro";
  ...
  
  String[] arguments = new String[] { "creazione" };
  process.execute(workItem, workflowSession, arguments);

  String expectedContent = "VdkVgwKey: http://www.mysite.com/its/a/news/lavoro.xml\n<<EOD>>";
  assertEquals(expectedContent, contentAt(ftpResource("/lavoro.bif")));
}

helper methods simply read resource content, stored at a given URL:

private String ftpResource(String resourcePath) {
  return
    "ftp://" + FTP_USER + ":" + FTP_PASSWORD +
    "@" + FTP_HOST + ":" + FTP_PORT + "/" + 
    ftpFolder.getName() + resourcePath;
}

private String contentAt(String urlAsString) throws Exception {
  return ContentUtils.readUrl(new URL(urlAsString));
}

then, was the turn for turning my local FTP server off, and using one for testing purpose. I have to admit, I tought it would have been easier, I’ve done this many times for HTTP, mail and JMS servers, but getting an FTP server started and stopped by a JUnit test turned to be painful! I played with MockFtpServer (hung waiting FTP data on port 0, no way to set any other port), then Apache FtpServer (had no luck with user grants on filesystem) and HermesFTP (gosh! it requires Spring for running!).

then found a lightweight and simple Java FTP server: AXL FTP Server (details for adding a Maven dependency are available here). its entry point it’s a Main method: i just managed to start a server thread on test setup and stop it on test tear-down (actually, on suite setup and tear-down).

// see src/test/resources/ftp/ftp.cfg
private static final String FTP_HOST = "localhost";
private static final String FTP_PORT = "2121";
private static final String FTP_USER = "foo";
private static final String FTP_PASSWORD = "bar";
private static final String FTP_HOME_FOLDER = "/tmp";

private static Thread serverThread; 
...

@BeforeClass
public static void startServer() throws Exception {
  serverThread = new Thread(new Runnable() {
    public void run() {
      ftpd.main(new String[] { "src/test/resources/ftp" });
    }
  });

  serverThread.start();
}

@AfterClass
public static void stopServer() {
  if (serverThread != null) {
    serverThread.stop();
  }
}

it was a matter of a few configuration files, which I put under Maven test resources folder (ftp.cfg for server config, and dir.cfg for users and filesystem grants).

this gave me confidence, really, adding a few more test cases. I could then start restructuring and refactoring code: extracting a brand new project, then removing duplicated logic, extracting classes and interfaces with clear names. in this process, as long as I was changing code, my understanding of the whole notification process improved.

in the end, I could easily move FTP integration test to the new project test suite, while unit-testing original code for workflow process with test-doubles (in-memory fake objects and mock objects). more unit tests were added, along the way, also.

well, it was really not legacy code, not yet! it was lack of automatic tests. this gave me the opportunity to apply Feathers’s process: take a picture of current behaviour, before starting refactoring code.

as usual, switching to a new project, i like to perform a code review on my own or while pairing, looking for un-emerged design or simply for duplications; that’s really the funniest thing for a design-maniac like me. sometime nice hints do come, sometimes don’t (probably because the design is good enough given actual goals).

so, last week i’ve moved to an existing codebase, and while pairing i noted down on paper some complexities i found on code; later, with all green bars, we tried to simplify a bit. what mainly come out was a revisited model-view-controller triad for a reporting system.

my preferite part was the formatting stuff: some nested complex loops became a composition of Formatters, each taking care of a small amount of rows (details, total) or columns (descriptions, calculations, and the like).

as we started a new story, i paid attention to how much code was reused: not as much as i supposed, and that was really a shame, even if code was clearer. today was our payoff.

we found a bug on a report. given a collection of aggregates, we expected to see details for first aggregate, then a summary for its total, followed by details for second aggregate, then second summary, and so on. instead, result was all details followed by all summaries. there was something wrong with how Formatters where composed. fortunately, that wiring was in one single point.

so, it turned out that it was just a matter of composing Formatters in a different way. from today journal:


we fixed a bug on rows order moving from a composite with two section formatters (details, summary) to a section formatter with a composite (details, summary). flexibility powered by OO.

day by day, i’m learning the difference between flexibility and generality (as Wake’s Refactoring Book helped me understand).

moreover, it now seems it’s also easier for us to estimate tasks, having clearer boundaries and reusing code. and, off course, knowing more about domain logic.

being honest, there’s a lot of noise due to Java generics! if only we could use .NET implementation..

Make it clearer

March 29 2009

good naming is first step for understanding domain you work into.

today i was looking at a session foused on learning from the master, in any creative field. Philippe pointed out some good advice from Beck’s “Smalltalk Best Practice Patterns” book; first of all is “good naming is not important, is crucial”. choosing a good vocabulary, focusing on roles, is step one to conquer complexity.

ok, but when is the right time to choose names?

Beck’s book, as Philippe states, teaches how to make good micro-decisions, every few minutes, while coding: that’s what Beck later called “implementation patterns”. yep, but coding is not the only right time to name things.

refactoring is another.

when i first studied Refactoring book what was clear to me was the distinction between refactoring as a noun (a modification which improve design but doesn’t change behaviour) and refactoring as a verb (the activity of modifying design). then, question is: when do you refactor, and what is the goal? let’s see..

last week, at least twice, my pair and i were in the middle of understanding which object was in charge of some new functionalities. when we found a “good” place where to start, first step was extracting a bunch of code into a new class to better focus on existing behaviour. moving code let us reflect on roles: does the “old” owner still have the same role? and what about the brand new class, which is its role? names should be choosen accordingly. refactoring can be applied before adding new functionalities, in order to make it simpler to add.

finally, a very good advice is to refactor code while doing code reviews. sometime, i found very useful to checkout a codebase from scratch, start looking at some code i don’t master and rename classes and methods as i understand roles. useless code and duplications emerge during this process. same strategy should be applied while pairing, after each green bar.

last advice for today: kids, don’t do this at home without an automatic tests suite!