how testing can improve legacy code design

Posted on by
Reply

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.

Great Developers Are Not Afraid to Experiment

Posted on by
7

As a moderator on the JavaRanch, I often come across people asking “What would happen if I executed the following code”. Many times the author of such posts can answer the question by copying and pasting his/her code into a Java main() method and running it. Some might chalk these posts up to being lazy, but, clearly, taking the time to write a post on a message board – often signing up as a member for the first time – takes some amount of effort as well. With that, I’m going to be go with the assumption developers avoid experimenting with code because they are scared or unsure of their own knowledge. Besides which, if it is a matter of laziness, there’s not much advice I can give except to say “Don’t be lazy”.

Experiment

Why is experimenting with code can be scary

In my experience as a teacher, development lead, and moderator I often come across developers who are unsure of their own knowledge. Often times they don’t fully understand what it is the code is doing and are afraid to experiment for fear it will either demonstrate their own personal weakness or harm the existing code. To them, I believe that experimenting is most crucial, if only because some of the doubts and questions that linger in their head can often be answered in an surprisingly short amount of time. If you are staring at a piece of code, puzzled by what you do not understand, take my advice: Step back and play 20 questions, ask yourself “What are questions I have about this code that can be answered with a simple yes/no?” then set up test code to answer each question. Once you have your answer, your doubt about the application should vanish, replaced by first-hand knowledge of what is going on. Keep in mind that sometimes these experiments lead to even more questions, but that’s good; it’s part of the learning process. In those cases, perform even more experiments on the code.

What is an experiment?

What constitutes an experiment? Oftentimes, it involves just writing a short line or two of code, then writing a logging statement, or, more commonly, if you don’t have a logger, a System.out.println() statement that displays the value of some variable or object. For example, if you don’t know why a method is behaving a certain way, add a dozen output statements throughout the code so you can follow two things: 1) the path the code is taking and 2) the value of the data throughout the code. Many times, you may be staring at a section of code, wondering why it’s not working, only to find out that section of code is never reached at run-time. Experimenting can be about changing values and recording the inputs, but sometimes its just about outputting where/what you think a process is doing.

Some people will recommend a debugger for experimentation but I’m not one of them. Aside from often being unwieldy and confusing to use, especially for beginners, sometimes running code through a debugger can affect what it outputs. For example, in a server environment, remote debuggers can be especially difficult to use. Services may have transaction timeouts of 30 seconds, and pausing the code in the debugger can cause an exception to be thrown before the method is complete. If you like using debuggers, more power to you, but as someone who has used both output statements via logging tools and debuggers, I greatly prefer the logging tools. Primarily, this is because the output statements give you more of a trial and error structure to work with: either a test succeeded, or it failed, and the output is all there in front of you.

Stand-alone Safe Experiments

The easiest and safest experiments are those you create that are completely separate from any other code. In terms of Java and Eclipse, this is akin to creating a new Workspace and a new Java Project to run the test code in. Every developer should have a temporary, throw-away workspace like this to perform low-level Java tests with. Simply create a file which in some way asks the question you want answered, and execute the program to evaluate the results.

Safe Experiments within Existing Code

Let’s say the code is highly framework dependent, such as often is the cases with J2EE and database-driven applications. For example, creating a temporary workspace to house your test case may be too cumbersome to implement. In such a case, you can run the experiment inside your existing code provided you are careful and follow some general guidelines:

  • If you are working with code that is backed by a repository, such as Subversion, CVS, ClearCase, etc, make sure your experimentation code does not get checked in or you may end up with applications such as this, this, or even the impossible (the last one). The DailyWTF has literally thousands of such cases. It is perfectly fine to experiment, just be careful to cleanup when you are done!
  • If you are working with code that is *not* backed by a repository, then install a developer repository! I cannot tell you enough the power and value of using a coding repository for all development work. In lieu of that, though, you should just make a copy of the project and/or workspace and experiment with the copy, keeping the original intact.
  • If you are working with code that connects to a database, make sure it’s not one other developers use. Making changes to a shared database as part of a test could affect other developers, so, if possible, you should have your own local copy of the database. This does not mean you should have a production database, but merely a copy of a QA or Development database

Final Thoughts
In my experience, one of the things that separates a great developer from the rest of the pack is that the great developer fully understands the code they are writing. When a bug appears (even great developers can cause bugs), this person often has a good idea where to look for the problem right away, and save valuable support time. Ultimately, if you ever find yourself mystified by your own code base, run some experiments and learn why things work the way they work. You’ll be a better developer for it!

The Joy of Null: Continued

Posted on by
Reply

In Part 1 of The Joy of Null I discussed a variety of ways null-equivalent values make it into the software design. Often times, developer laziness or immutability of the database tier drives many developers to insert values that simulate null values, rather than using a database null itself. In this second half, I’ll talk about other ways null-equivalent values arise as well as problems associated with using them.

Null

Part 1: Revisited
The most consistent comment I saw for why some developers choose to use empty strings instead of null values was for performance reasons. As I’ve mentioned on this blog before, database query optimizers are often a loose association of greedy algorithms and indexes, and its certainly possible a DBMS may perform better when a field is an empty string rather than a null, but I doubt its a general rule. In fact, from a theoretical standpoint, testing whether a field is null should be faster than comparing two strings. Consider that testing whether a field is null is a binary test, it either is or it is not null. Alternatively, testing whether a string is empty, or in the more difficult case is a set of blank spaces equivalent to an empty string, requires a string comparison which, depending on how it’s implemented, could be slow.

I think when people consider performance, they might be referring to the fact that no string = null in a database. In order to query on null, you must use “IS NULL” syntax since (x = null) returns false for all values of x, null or otherwise. Yes, having nulls and empty strings in a query could complicate the logic, but your goal should be to remove the empty strings, not the null values.

Part 2: Data Ambiguity
The first problem with null-equivalent values is you may unintentionally have more than one of them. For example, if you consider an empty string to be a null-equivalent value, do you also consider a 1 or more blank spaces null-equivalent? If so, your software will have to formulate SQL queries with syntax such as “trim(widget) LIKE ””, or alternatively “length(trim(widget)) = 0”. Either way, you now have to perform a database function on every string in a table in order to determine if a value is present, whereas “IS NULL” should be a lot faster.

The second problem with null-equivalent values is you may intentionally have more than one of them. I’m going to refer to enumerated null values as a set of null values that all represent null, but might mean slightly different things. As one reader pointed out, you may have a DBMS that supports enumerated null values such as NULL_MISSING, NULL_REMOVED, NULL_UNKNOWN, etc. I have no objection to using enumerated nulls except that very few, if any, major DBMS systems support it due to the fact it would be difficult to get a group of developers and DBA’s to agree on an enumerated set of nulls that would work across all domains. With that in mind, the vast majority of times enumerated null values are used, they are set as strings in the database. Such as Name = ‘NULL_MISSING’. This has all the performance and string comparison problems of my previous argument, but one even worse – someone’s name/data might actually match a null equivalent value. Your system would be a lot more prone to SQL injection if such a thing were allowed and would require constant conversion between the enumerated value and a useful value in the UI, since you don’t want to expose NULL_MISSING to the user. Keep in mind, this includes alternative null-equivalent values such as using -1 or 0 for a positive numeric field. At some point -1 or 0 may be allowed, or accidentally displayed to the user as a real value. No matter how you set it up, enumerated nulls can often lead to bad data such as the issue with little bobby tables

Finally, there are many times null-equivalent values are used side by side with null values leading to unruly queries such as “WHERE widget IS NULL OR length(trim(widget)) = 0″. As most good DBA’s already know, disjunctive searching (using OR) in SQL queries can significantly hurt a query optimizer. Disjunctions are among the most common paths query optimizers will ignore because searching them is not possible in real-time. In short, if you are using an empty string as a null-equivalent value then you should use it everywhere and make the column not nullable. This will at the least simplify your queries and remove the disjunction. There are other ways to formulate the query above without disjunctions, such as using the negation of length > 0, but it still leads to complicated queries.

Part 3: Choose one
I know there are some developers uncomfortable with using database null or “IS NULL” in their queries, so to them I say, at least be consistent. Either use a null-equivalent value, such as an empty string, everywhere or use null everywhere. The ambiguity is when you allow both, which in some data models may mean different things. Overall, allowing both will likely cause a performance hit. As for which you should choose, null or null-equivalent, if you have strong reason to believe your DBMS will handle empty strings better than null values, then go with empty strings. I, on the other hand, will stick with database null values since they should be faster for the vast majority of queries.