Refactoring: VFP form calls external PRG, uses Publics for shared data

I’ve inherited another legacy VFP application and am updating a routine to use a new format required by my client’s customer. There is a VFP form that eventually calls a program to gather, format, and export relevant data. One of the first things I need to do is change the form and program so their code can be independently tested. Once that’s accomplished, I’ll change the routines to generate the new format.

The question is, as always with this common situation is whether to pull the code into a “blob” method in the form and change all variables to properties, or call the program from the form with parameters. I realized my usual inclination is to use the former because eventually I would be binding the form properties to properties on future business classes that I’d create down the road. It’s odd how I’ll suddenly think of a usual problem in a different light. This is a routine that’s run four times a year, the new format is needed soon, and the client wants to begin planning for a new application to replace this one. Visual FoxPro will probably not be the tool of choice, so there is no reason to build the Queen Mary when a dinghy will get us across the narrow inlet just fine.

Can you tell I’m prone to make everything into A Work Of Art? Maybe I’m changing after all.

Refactoring: Rename Method Example

Have you tried my refactoring challenge[1]?  Since I missed two days in my first week of the 100 Pushups challenge, I guess I can’t complain if you haven’t tried refactoring. Here’s another example that might prompt you to try it. (Refactoring, not push-ups.)

I’m preparing a simple utility so the code is stranger-stable. In prepping the code, the following lines gave me pause.

SaveOldCommand()
WriteCommand( lcNewCommandPrg )

When I have to stop, even briefly, and ask what code is doing, or worse, if I have to divert into another method, I smell a refactoring. At first, I read the code as “SaveCommand, WriteCommand.” Wha..aa? The “Old” in SaveOldCommand() explains it, but it’s buried in the middle of the function name. Similarly the explicitly named variable lcNewCommandPrg passed to WriteCommand is good, but, it comes at the end of the line. I can refactor this code to make it clearer. To do that I used Rename Method. (Whether or not this is the most compelling example, isn’t the point, you kibbitzers out there.)

In a nutshell, renaming a method starts by creating a new method with the new name. Then move the old method’s code into the new method. Finally, change existing calls to the new method. Stay with me: it works on legacy code, too. More on that later. Here are the steps:

  1. Test the code (baseline, right?)
  2. Create a new method with the new name, with the same parameters, and test.
  3. Copy the code from the old method into the new one, and test.
  4. Find all references to the old method and change them, one at a time, to the new method.
  5. Test after every change in step 4.

Yes. Step 4 is a good example of “And then a miracle occurs” programming. But, no fears, this technique is adaptable to more difficult situations. In this case, however, I know it’s safe to remove the old method. After I refactor, the code is:

BackupOld()
WriteNew( lcNewPrg )

It’s usually impractical to rename a method other than in early development, isolated utilities, or hidden methods. Legacy code is too hard to search thoroughly and is rarely testable with the degree of assurance that’s required. If code is a control or application that is used by customers I cannot change method names at all. However, I can safely create new methods and change the old ones to call the new.

Consider the following changes to the steps above:

  1. Test the code.
  2. Use Extract Method to move the code from the old method to a new, better named method, and replace the old code with a call to the new method. Test.
  3. Change existing references to the old method, testing after each change for as long as you can stand it.
  4. Test.

“What’s the point of just adding another line of code to execute,” you ask? Consider a class that you will continue to use, or a class that customers use. Even if old code still calls the new method, a more clearly named method will be easier to work with, and the code that calls it will be more readable. Also, you might use BindEvent (VFP) to bind the old method to the new.

For example, years ago I adapted a Visual FoxPro foundation label class to send email. The original class used a method named something unhelpful. I’d added hooks so instances and subclasses could manipulate the email before it was sent. As I used the class in different projects, I wanted a more clearly named method, and so created a SendMail method.

The trouble with removing the old method was that I used the class in several different projects, and while I could have used Agent Ransack to find code references (this was in VFP 7.0, which doesn’t have Code References), I didn’t have time to find, replace, and test all the changes needed. Plus, I really just wanted to make it easier to use in the future. Otherwise it worked just fine as it already existed. So, I chose to the second refactoring and used Extract Method.

Why do I mention this? I want to convince the skeptic that refactoring is flexible, doesn’t require a massive investment in time, and leaves code better but still stable. I’ll keep coming back to these facts time and again. I can’t imagine why anyone wouldn’t want to refactor whenever possible. But then, I can’t imagine why anyone would eat lima beans willingly, either.

[1]No, I’m not really delusional about the size of my audience, its interest in refactoring, or my powers of persuasion. I just like to pretend.

Refactor for readability: replace comments with assertions

Arnon Rotem-Gal-Oz graciously took time to comment on my post Code comments, b-aaaad. He pointed out his recent post Code readability – Documentation vs. Refactoring, which I’ve had marked to write about. I’d gotten lost in some of the 3.0 issues he write about, though.

I agree with Arnon when he says that “while both of these efforts can help satisfy a customer specific requirement for ‘comprehansive [sic] documentation’ they have very little value in making anyone understand anything about your code.” (Emphasis is his.)

IOW, instead of explaining code, make code explain itself. In my own post I suggest a one week challenge for any readers who remain unconvinced of how much of an improvement even simple and minor refactoring can make. I wrote about looking for comments in longish sections of code, and refactoring with Extract Method. Arnon addresses long sections of code, and he gives a detailed example of how to do it. (Even if you don’t code in C#, it’s instructive.)

I find I’m most often refactoring long sections of code, and that Extract Method is the commonest technique I use. Comments are a useful hint in other situations, though, such as when they state a programming assumption. Instead of commenting assumptions, program an assertion.

It’s easiest for me to find examples in Visual FoxPro code because of its untyped variables, and because parameters are optional. VFP programmers are accommodating folk so we sometimes try to make sense of unexpected parameter types. For example,

LPARAMETERS toObject,tcName,tvClass,tvClassLibrary
IF TYPE("toObject")#"O" OR ISNULL(toObject)
RETURN .NULL.
ENDIF

It was only after programming in C# for a time that I realized this spirit of cooperation was flawed. Why? Frequently these functions change behavior depending on what is passed: sometimes subtly, sometimes not so subtly. It’s also confusing to the programmer (even if it’s just our future selves) who is trying to understand either the function or a mysterious error that occurs down stream because we’ve tried to protect the programmer from themselves. A non-VFP programmer expressed disgust with this co-dependent style by saying “sometimes an error should cause, uh, an error.”

I’m a convert, and I’ve long since stopped this practice except in rare and exceptional circumstances. Instead, I let bad parameters throw errors. I have an Intellisense script now that I use whenever writing a function or method. The IS script inserts the following code when I type “badparms”:

Assert Vartype( ) = "C" ;
Message "Did you mean to pass a bad parameter to " + ;
Program( Program(-1) -1 ) + "?"

(Obviously I specify the parameter in Vartype() and change the type (“C”) if need be.)

If I pass a bad parameter and it makes it into the client’s hands, the program will throw (and log) the error. When I run the code while debugging or testing, the Assert will help me debug the places I call the function incorrectly.

So, while you’re looking for comments (hello, hello, is this thing on?) look for ones that you can replace with an assertion. It will document the assumption and help you debug.

Refactoring: Step back to definition

Arnon Gal-Oz, an absolute rock star of software engineering, cautioned me with regard to the common misconception of refactoring. His point is well taken, especially since the misconception was one I held for a couple of years, before I stumbled upon the correct (and far more useful) definition.

In short, refactoring is not rewriting, nor is it intended to fix bugs or optimize code. Of course it might, as a side affect, but I think it’s far better to entirely ignore those otherwise laudable goals when one refactors.

Refactoring is quite simply imposing good, or even just better, design on existing code. Period. Full stop. End of sentence…it would make a good telegram, wouldn’t it?

For something so simple it’s surprisingly powerful, not to mention cost effective. It can even save on psychiatric bills. Really! :-)

It’s powerful because changes are made using a known methodology in small steps within a framework of continual tests. Since the code worked before you started, and since you will have tests to run at each step, you will know immediately when a change has gone wrong and what is at fault.

It’s cost effective because it makes changing features or behavior easier to implement and since you’ll already have a good test for the refactored code, changes will be easier to test. You really will spend less time debugging new code or new bugs later if you refactor now, even if you don’t have an immediate need to do any new programming right away.

Even better you can start small and convince yourself. You might even be fortunate to work in an environment that has automatic refactoring such as Java. But the benefits still apply to those of us who have do manual refactoring.

Using #Define statements in refactoring

I had the good fortune to present on the topic of refactoring at the 2006 Great Lakes Great Database Workshop (GLGDW) hosted by Whil Hentzen. In this presentation I discussed using localized #Define statements (FoxPro's preprocessor directive) to implement the refactoring pattern "Replace Magic Number with Symbolic Constant."

#Define statements can also be used to hide a calculation that by itself doesn't communicate clearly what, exactly, the calculation is. For example, the code I started with looked like this (somewhat simplified):

If This.Value < Date() - 100
  This.Value = This.Value + 100 * ( Int(  Year( Date() ) / 100 ) )
Endif

This snippet of code is (smelling) ripe for Replace Temp with Query. Rather than going to the bother of creating a custom method just to hide the trivial complexity of this calculation, I decided a local #Define would work very nicely, and it even makes the code more efficient. Even though it's not important in this case, and even though refactoring doesn't explicitly improve optimization, this is a good example of how refactoring can result in more optimal code as a side affect. Here's the snippet refactored:

#define lnCenturyNow 100 * ( Int(  Year( Date() ) / 100 ) )

If This.Value < Date() - 100
  This.Value = This.Value + lnCenturyNow
Endif

And, yes, I do realize I still have a "Magic Number," but that'll have to wait for a more pressing reason to change what's clearly (to me) a century.

Refactoring

I've fallen in love with the technique of refactoring since it promises (and proves!) a sensible approach to clean up code that otherwise would drive me batty. I'll use these pages to periodically post on the subject as I meet examples in my own programming that I think the community might find amusing, horrifying, or possibly useful. I program in both C# and FoxPro. HTML, too, if one can consider that a language in addition to being a specification. So my posts will likely relate to different languages.
I'm grateful beyond my poor skill with words can express to Martin Fowler, et. al., for his luminious book Refactoring: Improving the Design of Existing Code.