Thursday, June 16, 2011

Baseline Steps

Today I want to talk about modifications we make before we begin explicitly refactoring.

1: Pull each routine into its own file.
Like all modifications, this has pluses and minuses. The major minuses are:
a) There are more routines to keep track of -- using version control, this shouldn't be an issue
b) Make files-- If your program had been a monolithic beast before you started working with it, you may have been able to avoid the special joy that is MAKE. As they say, it builds character....

The advantage here is locality. Having each routine in its own file lets us find something on line 127, instead of searching for line 12097. The more we work with the code, the more powerful it becomes to have a small, clear area in which to focus our attention.

2: Turn the compiler warnings to maximum, and clean up everything.
This is another pain point. When I first applied this to my most recent project I had well over 200 things to fix. Mostly these fall into one of 2 classes:
A) Variables that are declared but not used. These are completely benign. Delete each one and move on. No fuss, no muss.
B) Variables that are not properly initialized. This is the first class bugs that we encounter. Everything should be explicitly initialized. Yes, I know that it is possible to use compiler switches to have uninitialized variables set to 0. That works, until you switch compilers or change optimization settings, or .... And then you get gibberish results, usually the night before a conference presentation, so that you aren't sure whether any of the results are trustworthy.

The basic principle is that we want the code to convey _ALL_ of the information about what the routine does. Relying on compiler settings is another way letting important information slide out of the code.

But the real reason for getting to the compile clean is that when we change the code, a new warning will stand out in clear relief, rather than getting lost in a blizzard of old, non-information warnings. Having the ability to draw our attention to an unintended change, as close in time to when that change was made, is invaluable.

3: Insert IMPLICIT NONE at the beginning of every routine.
Again, the goal is to make the code communicate as much as possible. The implicit variable types made sense in the late 1950's. They don't today. Using IMPLICIT NONE forces us to clearly how we intend to use variables.

The biggest advantage of IMPLICIT NONE is that it it removes a class of errors. Under implicit typing, if we misspell a variable name, the compiler silently creates a new variable. IMPLICIT NONE lets the compiler help, just like a spell checked does for a document.

4: Use KIND to specify the precision you require.
The specification of precision is another way of making your intent clear. The whole point of KIND is to facilitate updating and porting code. Take advantage of it.

5: Add INTENT settings for every argument.
INTENT is another way of letting the code communicate. Every argument can be specified as read only, write only, or read/write. This is another point may seem like overkill, but as we start to restructure the code, the added information to the compiler will again help it draw our attention to the exact point at which we've change the code's behavior.


All of this may seem like busy work, and who needs that? But I've found that taking the time to lay a solid groundwork pays handsomely, because it give quick feedback when a change has an unintended consequence.

Setting the Stage

Today I want to three preparatory steps that are vital to actively working on code. My belief in these comes from many painful experiences.

Step 1: Install a version control system and use it.

THIS IS VITAL. About every two weeks I have a situation in version control this saves me major heartache. As the saying goes, "Learn From My Fail."

Presumably the code you are working on is of value or you wouldn't be wasting your time fixing it. In the process of re-working the code, we are going to apply a lot of small changes. Every so often, a change will have an effect that we didn't foresee. The feeling of terror when the compile is fine, but the program suddenly starts giving screwy results is one I don't wish on anyone. In the best case, you still have the editor open and can undo your way back to a correctly functioning version. If not, you are faced with two unpleasant choices; toss all of your work to date to go back to the original program, or try to debug your way from the current version to one that works.

Version control software gives you a third choice: check out the most recent version that did work. This may mean discarding an afternoon's work, but I've found that it is almost always better than trying to debug my way out of a hole.

There are several systems that are: 1) free, 2) easy to set up and use. Probably the big 4 these days are CVS, Subversion, git, and Mercurial. In a Linux/Unix environment, you probably have a couple of these options pre-installed. CVS and Subversion have the simplest day-to-day use for someone working alone; git and Mercurial shine in collaboration within a team. Which you use is less important than using something. All have nice online tutorials, and all have books dedicated to them (if you are a book person, cruise over to look at the books on the Pragmatic Programmer site for a couple of very readable introductions).

Step 2: Get a small problem that exercises most of the code.
Regression testing is a very weak form of validation. But it is absolutely vital to have some verification that we haven't broken the code. Unit tests are certainly preferable, but I often have to deal with a SUBROUTINE that is a "big ball of mud", several thousand lines with a cyclomatic complexity over 100. McCabe's original work suggests values of 5 or less, with the occasional routine allowed to be in the 6-10 range (provided that a special dispensation was obtained from a suitable religious official :-).

Writing a suite of unit tests for such a monster is beyond my tolerance for pain, and would effectively wind up being a regression test anyway. Taking a known problem with known good solution is my compromise between best practice of having good unit tests, and the unacceptable risk of working with no verification that I haven't broken anything.

You are going to be running problem this lots (probably a few times an hour). Anything that takes more than a couple of minutes is going to get annoying really quickly. Take time to pare the problem down to something that runs in under a minute. It is well worth the effort.

Step 3: Automate the verification of the test.
In the programs I work on, even a small example can generate several hundred pages of output. My enthusiasm for trying to eyeball compare two versions of this file is non-existent.

Automation is a must. The goal is something that says a terse "OK" if the file matches, and gives you come info if there is a problem. For simple files, the operating system's file comparison command is sufficient:

fc original_output new_output

This is the simplest thing and it works. If the files don't agree, though, the amount of output that scrolls by can make it bewildering to figure out the differences. You can pipe the output into another file:

fc original_output new_output > results

and then open results in an editor.

If you are using this lowest tech approach, I've found that the best next step is to fire up an editor that supports tabbed documents, and load both copies. Flipping back and forth will allow you to zoom in on the difference(s) you care about.

This approach fails miserably if you have time/date stamps in the output. The file comparison routine will flag every one of these as a "difference", although they are differences you couldn't care less about. We'll need a somewhat sharper tool to deal with this issue, and I'll discuss a dead-simple one I use in a future post.

Summing Up:
So, the preparatory steps are:
1) Install version control and import the original version of the code. This serves as a powerful "undo" for your changes.
2) Find a small problem that runs in under a minute. This will increase your confidence that you haven't broken anything.
3) Automate the comparison of old v. new output. This allow you to get quick, simple feedback on whether you have broken something. The sooner you know, the easier it is to fix.

Raising Visibility in GLOBAL, part 1

One of the key "features" of Fortran is the global, COMMON block. The major plus of the COMMON blocks is that they allow us to write SUBROUTINEs and FUNCTIONs with relatively short lists of arguments. All of the detailed information passes back-channel in the COMMON block.
This is also the major down-side of using COMMON blocks. It is devilishly hard to figure out where variables get set to certain values. In "Working Effectively with Legacy Code", Michael Feathers describes a similar problem as "Sensing--We break dependencies to sense when we can't access values or code computes." Here, Feathers is referring to things we actually can't see, like whether a socket was correctly opened.

To make this work, we need to apply three preparatory refactorings. These are small, but depending upon the

The problem of COMMON blocks is related, the problem of Visibility. We can't see the flows of information, making it impossible to reason about the code. Today I want to describe three techniques that I use to deal with problems of visibility.

1) Pull COMMON blocks into a separate MODULE. Once this is done, delete the COMMON statement(s) from each of the effected SUBROUTINEs/FUNCTIONs, one at a time, and insert a USING statement to bring in the MODULE. Next, using the compiler, verify that the COMMON is actually needed. If you did the earlier refactorings of:
  • Introducing IMPLICIT NONE into each routine
  • Cleaning up all warnings
Now this bears fruit. Removing a COMMON block will yield a set of compiler errors that tell you exactly which variables resided in that COMMON.

At this point, you will probably find out that the COMMON statements you had were, indeed, needed. Not to worry; the information we gather here will be put to good use in later steps.

The other difficulty may be name collisions across the COMMON blocks. Fixing this may seem like more busy work. I would argue that, given the nature of COMMON blocks, naming different variables the same thing is likely to cause (or reflect) confusion in what concepts the separate variables are meant to represent. Flushing that out is part of being able to work meaningfully with the code.

Another approach can be more useful, depending on the exact circumstances. Pull each COMMON into its own MODULE, and include it only where it is needed. This is most useful when we have a few COMMON blocks that are essentially independent of each other. I list this as a secondary strategy not because it is less useful. it is more useful. In my experience, finding code with relatively independent sets of variables is uncommon (pardon the pun). The reasons are two-fold:
  • Older Fortran compiler design was generally based on the idea of memory overlays. In the tight memory configurations of the time, routines would be pulled out and replaced in the same location by the next routine. This swap could include COMMON blocks that were not used by the new routine. By putting all variables in a single COMMON block, we guarantee that there memory will not be swapped and possibly corrupted. Modern OS design is generally stack-based (indeed the language standard is moving in that direction), making this less of an issue.
  • The second, less common reason is that separate blocks generally flows out of a mindset of strongly decomposing the variables into pieces associated with certain routines. This line of thinking is much more object-oriented than was typical of early Fortran.
2) Pull your user-defined types into a separate file, named something like XXX_TYPES.inc. This is a perfect place to stash your SELECTED_KIND definitions that you are using for KIND specifications. Including this file will be less intrusive than specifying the SELECTED_KIND in each and every file. More importantly, it will guarantee that the types specified are coherent and agree across routines.

Overall, today seems like a big step backward. We've taken individual COMMON blocks and essentially created global data out of them.