Re: refactoring

From:

Patricia Shanahan <pats@acm.org>

Newsgroups:

comp.lang.java.programmer

Date:

Tue, 07 Aug 2007 07:22:56 -0700

Message-ID:

<f99v83$1h4q$1@ihnp4.ucsd.edu>

Roedy Green wrote:

On Mon, 06 Aug 2007 22:03:10 -0000, andrewmcdonagh
<andrewmcdonagh@gmail.com> wrote, quoted or indirectly quoted someone
who said :

Feel free to post your original method if you'd like me to show you
what I mean using your code.

Here is the code I am trying to tidy up. Of course my mind goes blank
on all the other code I either tidied or attempted to tidy with a
similar "multiple outputs" problem.

....

        else if ( refWithSlashes.startsWith( "guide/" ) )
            {
            // not same as technotes/guides
            javase6 = false;
            localOld = false;
            localRecent = false;
            longDesc = "Sun's Guide to " + boldDesc;
            }

        else if ( refWithSlashes.startsWith( "products/" ) )
            {
            javase6 = false;
            localOld = false;
            localRecent = false;
            longDesc = "Sun's Product Info on " + boldDesc;
            }
        else if ( refWithSlashes.startsWith( "webnotes/" ) )
            {
            javase6 = true;
            localOld = false;
            localRecent = false;
            longDesc = "Sun's Release notes on " + boldDesc;
            }
        else
            {
            // eg. j2ee
            javase6 = false;
            localOld = false;
            localRecent = false;
            longDesc = "Sun's documentation on " + boldDesc;
            }
        return buildSunLinks( refWithSlashes,
                              localRecent,
                              longDesc,
                              javase6,
                              localOld,
                              fileBeingProcessed );
        }

extracting the giant If does not really make the code all that
clearer. Somehow breaking up the giant if should be higher priority.

Roedy, can't you hear the three flags and longDesc yelling at the top of
the lungs they don't have "We're a class! We're a class!"?

If I were refactoring that code, I would probably end up with an
interface, two or three implementing classes for the interface, and
another class for the flags and description. Possibly, the flags would
turn into an EnumSet.

The aim would be a loop something like the following:

for(DescAnalyzer analyzer : someArray){
   if(analyzer.matches(refWithSlashes)){
     return analyzer.getResults(refWithSlashes, desc, bolddesc,//other
parameters);
   }
}

where someArray contains things like:
new ReMatcher("^products/",
   false, false, false, "Sun's Product Info on ")

I'm not sure of all the details, and obviously have not worked out
identifiers, but I'm reasonably sure your big If should be a table
and a loop, and that a more relaxed attitude to creating new classes
would get you there.

Patricia