how testing can improve legacy code design

There’s no shortage of articles on how TDD improves the design of new code.  That’s all well and good.  But what about legacy code?

How it came up

This weekend, I had occasion to make a few enhancements to the email sending project at JavaRanch.  The one that got me thinking about the design was when I needed to add some logic to filter the e-mail list.  I was trying to allow specifiying the start and end index so we could resend to just part of the list if the process failed.  After all, you don’t want people getting two copies.

What I did

As the current filtering logic was in the BulkMailerProcess class, I decided to start there.  This class has a bunch of dependencies and isn’t currently tested.  Hello legacy code!  My first thought was that I would make my new filtering method package private and test that.  As I set out to write the test, I realized I needed to get access to the instance variable containing the list of e-mails.  Ugh.  This made me cringe enough to think about an alternate direction.

My next thought was that I really have a separate concept here.  I’m filtering e-mails.  So it one of the existing methods in BulkMailerProcess.  (It makes sure the e-mails are properly formed.)  Time to create a new class.  At this point it was easy.  My new class EmailFilter takes a list of e-mails and runs both filtering/cleaning operations on them.  It’s very focused and gets all that logic out of the main processing class.  I feel like I left the code cleaner than I found it.

The result

It certainly is more tested.  The method to clean the e-mails wasn’t originally tested since it was so embedded in everything else.  Now it is.  In fact now it is tested at 100%.  Without looking at the implementation of the method I copied in, I did write one test to verify the method gets called and at least does what it sounds like.  With this system I got to 96% coverage on the new EmailFilter class.  I could easily get to 100%, but that’s for another day.  The goal here isn’t to be perfect.  It’s to leave things better than they started out.

I left a comment in the code so nobody thinks it is more tested than it is.

/**
* Check valid format for email. This logic was moved verbatim from
* BulkMailerProcess class. Do not change without adding more detailed unit
* tests to verify behavior.
*/

What’s next

The only thing I’m less than happy with is the name of the class.  I started with EmailListUtil because I didn’t have a better name.  Then I changed it to EmailFilter.  But it’s not just a filter.  It’s doing validation too for cleaning the invalid e-mails from the list before filtering by index.  I’ll have to think about the name some more.

The other next step is to improve things a little more next time I touch the code.  I already have some ideas.  The key is to not attempt too much at once.  That would be overwhelming.  A little at a time makes it doable.

perpetual novice – years of experience vs skill

Most job listings ask for a certain number of years of experience.  Either in a particular technology or in the industry.  This presumes that the developer learned each year.

I read a quote about whether the candidate has five years of experience or the same year of experience five times.  I don’t remember where this came from, but I think it clearly expressed an important point – that we can’t just measure years.

Upon re-reading the “Software Craftsmanship“, I came across the concept/phrase “perpetual novice.”  This is exactly the idea.  If one never picks up best practices and software development habits, all that time could be spent repeating inefficient ways of doing things.

I’ve seen this upon interviewing as well.  Some people with “X years experience coding Java” look like they can’t code their way out of a paper bag.  I’m thinking this is related to the “perpetual novice” concept.  These people figure out everything from scratch and repeatedly solve problems the industry has surpassed.

What’s dangerous is that such developers don’t consider themselves novices anymore.  Someone new to the field or right out of college realizes they are a novice.  They ask questions and realize there is much to learn.  “Perpetual novices” think they know quite a bit and are less likely to ask a more experienced person.

And worse is when the real novices learn from perpetual novices – the opportunity to learn better ways is lost and the cycle continues!

Have you encountered any perpetual novices in your career?  What do you think makes them stand out?

process maturity and the schedule

“When we started our project nobody bother about <activity A> because we had very tight delivery schedule.”

Every so often, one sees a statement of this form.  I think it points out that “Activity A” is not an essential part of the process.

It could be because ‘Activity A’ isn’t perceived as having enough value yet.  I think it is a sign of process maturity when this statement doesn’t occur.  For example, I remember a time when we threw tests and the automated build out the window for emergency bug fixes.  Over time, we appreciated the value of these tools and especially the safety net they give us.  I don’t think anyone has thrown out these parts of the process in years!

What really hits me with the quote is that, ‘Activity A’ is perceived as having some value since the author of the quote feels compelled to justify why it wasn’t done.  This is like justifying not brushing ones teeth!

As a process matures you get different levels of value:

– not there yet – don’t even feel the need to make excuses

– immaturity – do the activity sometimes but throw it out the window when things get tough

– maturity – do the activity always

This activity could be anything.  Seeing it get done for emergency production fixes really shows me it has been fully embraced as part of the process.