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.

Advertisements

5 thoughts on “Using #Define statements in refactoring

  1. Hi, Christof- I’m honored that you’ve taken time to read my post. A few comments in reply.

    First, there are three refactoring tenets that apply here.

    1. Refactor code that works. So, I did, indeed, have an intermediate step looked much like your suggestion.

    2. Encapsulating calculations in methods improves readability and reusability

    3. Replace lazy temps (variables)

    So, once the code worked, and before I left it, I saw the possiblity to apply the refactoring “Replace Temp with Query,” which would have looked something like

    This.Value = This.Value + this.CenturyNow()

    With the addition of a method call.

    However, that was overkill, thus I decided upon the #Define. In my opinion my result accomplishes both goals: it’s more readable than the original and the calculation is more reusable.

    Point #3 above is controversial, I know. Since there is a competing notion about writing good code that one should leave debugging code in, as was pointed out to me during my presentation at GLGDW. As with most of life, there are times when two good ideas with rub each other the wrong way. If this example was of something more complex, then I would have used a method instead of the #Define, which would give me a debugging hook.

  2. That’s interesting. Why would say that adding a method call is overkill?

    Is it, because it’s a method with a single line? Some Smalltalk developer say that any method with more than three lines is too long. ;-)

    Or is that you see a performance impact (which is certainly there). When choosing between a more maintainable or a faster solution I’d always pick the more maintainable one as long as it is fast enough.

    In any case, I’m still not convinced about that #DEFINE. After all, with C++ they tried to discourage the use of #defines by adding language features like constants, inlince procedures, etc. that were just as fast, but can be validated by the compiler. One of the reasons for inline procedure was that C developers frequently made mistakes when defining a constant. You would run into the same problem:

    ? 2/lnCenturyNow

    What would you expect? The current century is 2000, so the result should be 0.001. If you execute this with a temporary variable or the method call, that’s exactly the result you get. However, after you refactored to use a macro, you suddenly get 0.4. Why? Because the expression now reads:

    ? 2/100*( Int( Year( Date() ) / 100 ) )

    What you meant to use is

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

    with parenthesis surrounding the expression. To me, that makes #DEFINEs a riskier solution. If I have to pick between a risky and an unrisky solution, I try to use the less riskier one.

  3. Christof-

    I expect that we will simply have to agree to disagree about the implementation I chose for this refactoring. I hate to bog down the interesting (to me) ilustration of refactoring with a quibble about #Defines. For example, refactoring guides one to implementing, IMO, “good enough” solutions. “Good enough” being relative to the time available to refactor, the criticality of the code being refactored, and the known goals for the code.

    You’ll just have to trust me when I say the method was overkill…at this time, in any case. If it became useful at some point, it would be a snap to do by applying yet another refactoring. Which would depend on the new goal. I’ll write about this soon. Thanks for the stimulating conversation. :)

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s