Beefy Boxes and Bandwidth Generously Provided by pair Networks
The stupid question is the question not asked
 
PerlMonks  

Re: Refactoring just to refactor?

by davido (Cardinal)
on Jun 28, 2019 at 05:19 UTC ( #11102060=note: print w/replies, xml ) Need Help??


in reply to Refactoring just to refactor?

The formatting is fine, but to me reformatting is not refactoring. Refactoring is using different programming constructs, redesigning abstractions, splitting apart subroutines that grew too big, too hard to test, have too many side effects, or are doing too many things, factoring out common code into base classes, helper subs or modules, shifting over to more appropriate algorithms, that sort of thing. Reformatting is just reformatting; a change in whitespace is inconsequential except to the people who have to read the code. Reformatting is not refactoring.

When to reformat? When you think you've found a formatting technique that will make the code more legible. But in a world with version control, beware that simply deleting a trailing space will mostly just serve to add nose to pull requests making it harder to see the forest through the trees. And will possibly mislead people who come along in the future looking at the code to believing that you modified it when it was only reformatted. For example, I occasionally have to do some large scale moving of code from one layer to another, and in so doing suddenly find my name next to the git blame. A year later someone comes along and asks me questions about it thinking I wrote it. This risk makes sense for substantive changes, but probably doesn't make sense if it's just a matter of tidying up whitespace.

On the other hand, if tidying up whitespace seems like something you wish to do, do it as separate commits from any substantive changes, so that it's easy to identify the important things that changed, versus the shape of the code's formatting.


Dave

Replies are listed 'Best First'.
Re^2: Refactoring just to refactor?
by Tux (Abbot) on Jun 28, 2019 at 06:20 UTC

    ++

    I fully subscribe to that. Well said!

    One more note I cannot resist here: reformatting/restyling sometimes "improves" readability for some where it "obfuscates" for others. When in a team get consensus.

    Some changes might yield the same underlying code and thus seem to be just styling:

    $ perl -MO=Deparse -e'$a = 1 unless $b;' $a = 1 unless $b; -e syntax OK $ perl -MO=Deparse -e'$b or $a = 1;' $a = 1 unless $b; -e syntax OK

    but where some see a change from first to second or vice versa as a readability improvement, others might percieve this as plain obfuscation. Not all minds work alike.


    Enjoy, Have FUN! H.Merijn
      When in a team get consensus.

      And with that in mind, use a .perltidyrc and hook it into the commit workflow. Do this even when not in a team for (a) practice and (b) self-enforced consistency. It then makes later global reformatting a doddle too.

      I agree with everything davido said and the other replies to him.

Re^2: Refactoring just to refactor?
by GrandFather (Sage) on Jun 28, 2019 at 06:05 UTC

    I strongly agree with your advice to do code tidying as a separate commit from any substantive changes. I often tidy as I work on code, but using Mercurial and TortoiseHg I can cherry pick commits so it is generally easy to pick up all the "tidying" related changes and commit those then preform the "substantive changes" commit. Indeed, I moderately often cherry pick commits for substantive changes so I can highlight parts of a piece of work.

    I avoid Git to the extent possible so I don't know what it offers for cherry picking changes, but I'll bet there is something in its slew of cobbled together tools that allows identifying and selecting changes to commit.

    Optimising for fewest key strokes only makes sense transmitting to Pluto or beyond
      I avoid Git to the extent possible so I don't know what it offers for cherry picking changes
      It's called
      git cherry-pick
      :-P

        :-D

        Optimising for fewest key strokes only makes sense transmitting to Pluto or beyond
Re^2: Refactoring just to refactor?
by Lady_Aleena (Curate) on Jun 29, 2019 at 21:28 UTC

    Sorry for the misnomer of the original post. I will keep this in mind if I use version control in the future.

    No matter how hysterical I get, my problems are not time sensitive. So, relax, have a cookie, and a very nice day!
    Lady Aleena

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://11102060]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others chilling in the Monastery: (6)
As of 2020-11-30 23:27 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found

    Notices?