in reply to Re: Refactoring technique?
in thread Refactoring technique?

It really doesn't matter where the input values come from, but what is the purpose of the subroutine.

You're right! Until I can give doStuff( ... ): a proper name, the parameter names are always going to be a compromise.

The problem is that the 6 blocks of code that will be replaced by the calls to doStuff(), do an aweful lot of stuff; so understanding all the stuff they do sufficiently to give it an appropriate name is quite a challenge. More so as I'm working in a language I'm only barely familiar with, and the author has made some ... what you might term politely .. dubious choices.

Eg. The code is littered with calls to a function called _OAV(), which is annotated with //Object, Attribute, Value. It looks like this:

function _OAV(oo,aa,vv) //Object, Attribute, Value
{ if (useSVG)
  { if (aa=="from")
    { with (oo) 
      { setAttribute("x1",vv.split(",")[0]);
        setAttribute("y1",vv.split(",")[1]);
      }
      return;
    }
    if (aa=="to")
    { with (oo) 
      { setAttribute("x2",vv.split(",")[0]);
        setAttribute("y2",vv.split(",")[1]);
      }
      return;      
    }
    if (aa=="color")
    { with (oo) setAttribute("fill",vv);
      return;
    }
    if (aa=="innerText") 
    { oo.firstChild.replaceData(0,108,vv);
      return;
    }    
    with (oo) setAttribute(aa,vv);
  }
  else
  { if (aa=="fill") oo.fillcolor=vv;
    if (aa=="stroke") oo.strokecolor=vv;
    if (aa=="stroke-width") oo.strokeweight=vv;
    if (aa=="visibility") oo.style.visibility=vv;
    if (aa=="from") oo.from=vv;
    if (aa=="to") oo.to=vv;
    if (aa=="x") oo.style.left=vv;
    if (aa=="y") oo.style.top=vv;  
    if (aa=="d") oo.path=vv;
    if (aa=="color") oo.style.color=vv;   
    if (aa=="innerText") oo.innerText=vv;    
  }      
}

[download]

Which is just horrible!

With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'
Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
"Science is about questioning the status quo. Questioning authority". I'm with torvalds on this
In the absence of evidence, opinion is indistinguishable from prejudice. Agile (and TDD) debunked

Replies are listed 'Best First'.
Re^3: Refactoring technique?
by MidLifeXis (Monsignor) on Apr 24, 2015 at 17:24 UTC

    Which is just horrible!
    Ewww. Agreed.

    It seems to me that the purpose of this is to dispatch the setting of attributes. Perhaps setAttrDispatcher( obj, attr, value ). Still ick. Longer (of course, that is hard not to be), and still confusing (where does useSVG come from, for example).

    update: small bit of cleanup and clarification

    --MidLifeXis

[reply]
      (where does useSVG come from, for example)

      Ah. That bit's gone from my version.

      The library supports both SVG and VML, and useSVG is a global that is set either manually at the topmost level or by detecting the useragaent. I'm not interested in supporting old IEs, so I've already stripped all the useSVG and VML stuff out.

      I've also separated out all the branches of the sub and at this point that sub reduces to:

      function _OAV( oo,aa,vv ) { //Object, Attribute, Value
    oo.setAttribute( aa,vv );
}   

function fromOV( oo, vv ) {
    oo.setAttribute( "x1", vv.split( "," )[0] );
    oo.setAttribute( "y1", vv.split( "," )[1] );
}

function toOV( oo, vv ) {
    oo.setAttribute( "x2", vv.split( "," )[0] );
    oo.setAttribute( "y2", vv.split( "," )[1] );
}

function colorOV( oo, vv ) {
    oo.setAttribute( "fill", vv );
}

function innerTextOV( oo, vv ) {
    oo.firstChild.replaceData( 0, 108, vv );
}

      [download]

      Most of those will be inlined back into the main code; with only the fromOV() & toOV() which set two attributes left to deal with. The current practice of concatenating the two values together in the calling code and then splitting them in the functions will go. Not sure beyond that.

      With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'
      Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
      "Science is about questioning the status quo. Questioning authority". I'm with torvalds on this
      In the absence of evidence, opinion is indistinguishable from prejudice. Agile (and TDD) debunked
[reply]
[d/l]
Re^3: Refactoring technique?
by flexvault (Monsignor) on Apr 24, 2015 at 19:50 UTC

    BrowserUk,

    WOW! Now I understand!

    So the new sub would be: 

    # my $WhoKnows = SeeOriginalAuthorForMoreInformation( $oo, $aa, $vv );
+ 
sub SeeOriginalAuthorForMoreInformation
{   my $Object = shift; my $Attribute = shift; my $Value = shift;
    ...
}

    [download]
    I guess the author wants you to be a miracle worker :-)

    It wouldn't be so bad if you could move 'to' and 'from' to a different subroutine. Good Luck!

    Regards...Ed

    "Well done is better than well said." - Benjamin Franklin

[reply]
[d/l]

In Section Seekers of Perl Wisdom