I've looked at Some thoughts around the "is Perl code maintainable" discussion, as well as other threads on Programming standards, peer reviews, etc. and have a related question.

When you need to enhance or debug code written by someone else, do you restrict yourself to the minimal change? Or do you find yourself cleaning up code, making it look more the way you program?

I've inherited code written by someone who wrote verbosely. I keep finding myself saying "I could make this so much smaller, so much more efficient".

Mind you, verbose programming can be very useful, especially in an environment where you don't know who may end up supporting your software. Dense programming could cause more problems, and would require extensive retesting.

Regardless of that, I'm curious: Do you give in to the temptation to make someone elses code look more like yours, or do you successfully resist it?

Replies are listed 'Best First'.
Re: Maintainance vs. Programming style
by moritz (Cardinal) on Jan 28, 2008 at 18:18 UTC
    When I make only small modifications, I try to adapt to the current coding style with respect to indentation, naming of variables (CamelCase or no_camel_case or whatever), placement of else (on a new line or not?) etc.

    When the code is horrible, for example no indentation at all (or only on two levels :/), one-lettered variable names etc., then I make my code look ok, and refactor the rest according to my time constraints.

    Of course that depends on whether I'll end up maintaining the code - the more I have to deal with it, the more it will converge to my notion of programming style.

Re: Maintence vs. Programming style
by Old_Gray_Bear (Bishop) on Jan 28, 2008 at 19:24 UTC
    Like everything else in this World: 'it depends'.

    If I am only fixing a bug in the code and there is already a Responsible Party (the Original Author, a System Owner, etc), I try to match the coding style, then send them the patch.

    If I have inherited this Hair-Ball, then I have to decide whether the code has a life span longer than the time I am going to be the RP. If I am only the place-holder while we 'end-of-life this Monster as fast as we can', I am comfortable letting sleeping dragons lie. If the Code will out last my curatorship, though, then I feel an obligation to spend time cleaning up the more egregious problems.

    I tend to the Verbose style, anyway, so that doesn't bother me as much as 'terse, economical, and uncommented' code. I have always tried to code so that someone on an 0300 Page-from-Operations doesn't have to think to much. The Poor Old Sod might be me, and I know how fast I think at that time of night on no coffee.

    So, the first thing I do is wade into the morass and document. My documentation comes in two parts: POD (or equal) and tests. I write up my understanding of the way the code works in the form of tests for the (all too often non-existent) test harness. Once I know that 'what I know' is really true, I write up a POD stanza to go in the code. After a bit, when I begin feel that I really understand that is going on, I start refactoring; Serious Refactoring. I try to spend an hour a day on my Pet; getting the Hair-Ball in shape isn't my primary job here, but I use the refactoring project as a relaxation at the end of the day.

    It gives me a certain pleasure to (over the course of six weeks) turn a 10,000 line monolithic block-o-code into a cleaner more maintainable structure. (Only 4,000 lines (including inline POD) consisting of a Base Module to hold the singleton variables, twenty-five Inheritors (one for each of the 'basic' functions), and four Helper/Common Utility Modules. Oh, and 3,000+ tests in the brand spanking new test harness.) Partly, it's because I don't like to have my name associated with slock, and partly it the innate desire to tinker. (And mostly it's the old Boy Scout ethic -- leave the camping site cleaner than when you found it. But...)

    I have performed this process maybe a dozen times in the past five years, and I just inherited another opportunity last month. I am going to have loads of fun in 2008.

    ----
    I Go Back to Sleep, Now.

    OGB

Re: Maintence vs. Programming style
by toolic (Bishop) on Jan 28, 2008 at 18:24 UTC
    I admit that I do refactor other's working code, but only if:
    • We are using a version control system,
    • The change is small and contained, and
    • We have a decent test suite.

    For example, one change I am forever making is to get rid of escaped quotes within a quoted string. I consider this hard to understand:

    print "I said \"Hi\". You said \"Bye\".";

    But, this is much easier to understand.

    print qq{I said "Hi". You said "Bye".};
Re: Maintenance vs. Programming style
by Errto (Vicar) on Jan 28, 2008 at 18:50 UTC

    If you're lucky enough to be working on part of a larger project or organization with defined coding standards, stick to those even if they disagree with your personal taste. People tend to get annoyed when they see you've checked in a two-line substantive change and 50 lines of renaming variables, moving braces, etc.

    As an example, many people on my team never learned the lesson about bind variables in DB calls. So if I'm working with code that has this problem, I'll fix the statements involved in the code I'm working on but I'll leave the other ones in the module alone, unless I spot an actual bug in them.

    Refactoring, especially within a module, is usually a good idea if it helps you introduce the change you need to make more cleanly. If I find that there's existing code that has two largely identical blocks and my change would require editing both of them, I'll try to combine them first so my new change becomes smaller. But I don't generally refactor code unrelated to the change I'm making even when I'm certain I'll do it better, just because it's not relevant at that moment.

    If the change you need to make is more fundamental, or if the existing code is so horribly fragile that it can't accept any further changes, then large-scale rework (if not rewriting) may be called for. But be careful with this. When you see what looks like incomprehensible weirdness in the existing code, remember that it may reflect incompetence or laziness on your predecessor's part, but it may also have been a necessary workaround for some unpleasant real-world scenario.

Re: Maintence vs. Programming style
by kyle (Abbot) on Jan 28, 2008 at 18:19 UTC

    One day I was in a piece of code, and I thought, "this is all scrunched together; a little space would make this a lot nicer." And I set about adding blank lines where I thought they improved readability. Later, I was looking at the revision history for the file, and I saw that I had basically undone the changes of the last programmer. That programmer must have thought, "this is all spaced out; I could see more of it if it were condensed..."

    Generally, I try to make the least change I can. Sometimes I find something that I think is so horrible that I do change it. In those cases, I'm thinking that the style is bad enough that it's no longer a matter of taste. Even then, I prefer to keep those changes separate from actual functional changes. I want future generations to be able to sort out the important stuff from me imposing my horrifying style on innocent code.

Re: Maintenance vs. Programming style
by dragonchild (Archbishop) on Jan 28, 2008 at 22:34 UTC
    Having broken working code through "obviously identical reformatting", I will never reformat code unless I am willing, capable, and permitted to take FULL OWNERSHIP of that code. For me, there is either "minimally sufficient changes" or "complete ownership" - nothing else and definitely no middle ground.

    My criteria for good software:
    1. Does it work?
    2. Can someone else come in, make a change, and be reasonably certain no bugs were introduced?
Re: Maintenance vs. Programming style (efficiency)
by tye (Sage) on Jan 29, 2008 at 03:42 UTC
    I've inherited code written by someone who wrote verbosely. I keep finding myself saying "I could make this so much smaller, so much more efficient".

    I don't find that writing easy-to-read/-maintain code makes inefficient code. This reminds me of a conversation from a few years ago at PerlMonks where I kept trying to figure out why someone insisted on making code smaller. It turned out that they assumed that smaller code was more efficient. That isn't the case.

    So I object, in general, to "more efficient" and especially to "much more efficient". Other than for quick hacks, I write efficient code that is "verbose".

    Now, if somebody is writing for( $i= 0; $i < @list; $i++ ) { ... $list[$i] ... }, then I'd likely refactor to for my $item ( @list ) [or, if $i is actually used besides for $list[$i], then for my $i ( 0..$#list )] when the time comes. But the main reason for that is neither compactness of code nor efficiency. Looping over lists is less bug-prone than other types of looping.

    Note that I'm not claiming that your inheritted code has been written efficiently. It is just that the phrasing made me suspect there might be a "verbose code is slow code" bias.

    One of the best routes when refactoring code, especially if you didn't originally write it, is to first flesh out your unit test suite so you are more likely to notice if your "improvements" change the behavior of the code. Writing and running more unit tests also helps you understand the code more fully and may even find some long-standing bugs.

    - tye        

      tye,
      This reminds me of a conversation from a few years ago at PerlMonks where I kept trying to figure out why someone insisted on making code smaller. It turned out that they assumed that smaller code was more efficient. That isn't the case.

      You mentioned that to me a little over 5 years ago when I had about 5 months of perl under my belt. In retrospect, I think my problem was two fold. The first issue I had was that I didn't take the time to understand what your code was doing so when it wasn't fast enough, I didn't understand how to adapt it. The second part is that I did know that fewer ops usually means less work which usually means faster run time. I was a bit naive about achieving the fewer ops.

      I try not to make that mistake anymore.

      Cheers - L~R

      I should have been a little more explicit about what I meant by "verbose", but I was too interested in what everyone else had to say.

      Some of the code I was assigned was written by someone who didn't understand how to restrict scope in Perl, or specify global information. As a result, every subroutine had 15 parameters. To say this made understanding his code difficult is an understatement.

      Another former collegue never used subroutines. *sigh*

      I appreciate your comments on "verbose code is not slow code" and will try to keep them in mind. It is a fallacy I have fallen into.

Re: Maintenance vs. Programming style
by jplindstrom (Monsignor) on Jan 28, 2008 at 22:58 UTC
    If you are a tourist in the code base, do the least intrusive thing possible. Definitely stick to any explicit or implicit coding standard.

    If you have inherited a code base, you will gradually get to know it. After a while you know the code well enough to form a vision of your own on how things should be, and molding the software to that vision can actually be rewarding since you get to see steady progress as you fix things.

    The book Perl Medic is an excellent read.

    /J

Re: Maintenance vs. Programming style
by LTjake (Prior) on Jan 28, 2008 at 20:56 UTC

    In the $work context, we have a standard .perltidyrc file that each programmer uses. Each person is free to program in whatever way they wish, but before checking in they should run the project through Perl::Tidy.

    --
    "Go up to the next female stranger you see and tell her that her "body is a wonderland."
    My hypothesis is that she’ll be too busy laughing at you to even bother slapping you.
    " (src)

      Unfortunately, on our current project, 90% of the "blame" in svn annotations for who wrote code is placed on the person who imposed Perl::Tidy on the (100-200k SLOC) existing codebase.

      Also, I don't trust Perl::Tidy to not break code.
        You of course sent examples to the author of Perl::Tidy? The same author who specifically says that it doesn't work for everything? Also the same author who asks for help in running down the problems found in the field-- Or is enough for you to say that you just 'don't trust Perl::Tidy to not break code'... Of course it might be the case that code that breaks Perl::Tidy is complex to the point that it needs to refactored anyway---hmmm, useful even when it doesn't work; hard to beat that!

        --hsm

        "Never try to teach a pig to sing...it wastes your time and it annoys the pig."
Re: Maintenance vs. Programming style
by Tux (Canon) on Jan 29, 2008 at 09:33 UTC

    As most posters here agree, if $work imposes standard style and coding practice, follow that! If I find colleage-code that doesn't follow these rules, I fix it and remind that person about the style we agreed and why. <rant>It cannot be denied that some people care less about small things like indentation and whitespace use than others</rant>.

    If I get a project as a new and only owner, the very first thing I do is rewrite every single line to adhere to my style. Not only will the new code be very consistent and beautifull layed out in my perception, but it has proven a great way to understand what the original author had meant with the code in the first place. By reformatting, renaming, and reordering, you get a very good grip on the complete project and you can add the comments of your findings while you are at it, giving the next maintainer (in case you die) an even better base to start from.

    Once style issues are `fixed', and of course all tests still pass, the first thing to do then is to put the project in git (fill in your favourite VCS) and find open/outstanding bugs. Now you understand the code, and have no excuse about not able to read it, fixing those should be easy^Wless difficult. Add tests for all the things that were unclear on this journey through code and for all the shiny new features you just added and release it.


    Enjoy, Have FUN! H.Merijn

      Bravo. I concur with every word.

      The only addition I would make is that if I have to fix or modify a piece of shared ownership/corporate code that is in a style radically different from my own preferred, then I will often go through the same "re-write and rename to my style" process in order to get to know that piece of code--on a private copy.

      Only once I am convinced that I understand what the code really does and it still passes any existing tests, will I attempt to fix or modify it. And once I have made and verified the changes, I'll go back to the original and attempt to re-make them, in the original style, minimizing the changed lines commensurate with doing the job right before check in.

      80% (figure varies) of the life cost of a piece of code may be in the maintenance phase, but I've seen studies that put up to 50% of that 80% as backing out incorrectly fixed bugs; fixing introducd bugs; or the backing out of features added that broke existing code.

      Hit & run maintenance costs. And the two primary causes are: 1) 'simple' changes made to 'simple' code that turns out not to be; 2) changes made on the basis of out-of-date comments. I suspect, but have never seen proof, that a third cause is attempting to minimise changes for the sake of VCS change history stats.


      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".
      In the absence of evidence, opinion is indistinguishable from prejudice.
Re: Maintenance vs. Programming style
by CountZero (Bishop) on Jan 29, 2008 at 13:25 UTC
    The more badly written a program is, the less changes I wish to make for fear that it will have unknown and unexpected effects. Of course, that means that I only want to refactor code that is already well laid-out. ;-)

    CountZero

    A program should be light and agile, its subroutines connected like a string of pearls. The spirit and intent of the program should be retained throughout. There should be neither too little or too much, neither needless loops nor useless variables, neither lack of structure nor overwhelming rigidity." - The Tao of Programming, 4.1 - Geoffrey James

Re: Maintenance vs. Programming style
by talexb (Canon) on Jan 29, 2008 at 17:42 UTC
      When you need to enhance or debug code written by someone else, do you restrict yourself to the minimal change? Or do you find yourself cleaning up code, making it look more the way you program?

    That depends. Do you want to get in and get out, or are you going to turn this into a career?

      I've inherited code written by someone who wrote verbosely. I keep finding myself saying "I could make this so much smaller, so much more efficient".

    Sure. And are there exhaustive tests that you can run to verify that your "cosmetic" changes don't make any of the tests fail? And what's the risk?

      Mind you, verbose programming can be very useful, especially in an environment where you don't know who may end up supporting your software. Dense programming could cause more problems, and would require extensive retesting.

    I have a confession to make. My code is usually pretty terse, but I suffer from over-commenting. So I'll write something brief, then spend the next five lines explaining what I just did (or what I'm about to do, since I put comments ahead of the code).

    I'm just terrified of coming back to my own code in six months to a year's time and being unable to grok what it's doing.

      Regardless of that, I'm curious: Do you give in to the temptation to make someone elses code look more like yours, or do you successfully resist it?

    Nope -- I get in, make the smallest possible change, look over it *very* carefully, do as much testing as possible, and get out. I don't even muck with the indentation.

    You've heard the phrase "The road to hell is paved with good intentions"? Many of those good intentions are a variation on "But I only made a small change to the code! It shouldn't have changed anything!"

    Great question.

    Alex / talexb / Toronto

    "Groklaw is the open-source mentality applied to legal research" ~ Linus Torvalds

Re: Maintenance vs. Programming style
by sundialsvc4 (Abbot) on Jan 29, 2008 at 17:42 UTC

    If your task is to enhance-or-debug, then by all means limit yourself to that. But obviously during the process of doing that you must also make “triage decisions.” That is, “is this corpse worth saving?”

    Many things that stink are not dead, however. (In fact, quite a few things that I have written over the years acquired quite an ex post facto “smell” even to me.) You don't need to rewrite all that. Doing so won't necessarily make it any better at all.

    All of us have worked in, or with, shops that had “famously bad code.” It was this-or-that module which was so constantly broken that people became adept at – even proud of – nursing it along. And you just gotta make that “dead-horse decision,” pull the trigger next to Old Dobbin's head and replace the damm thing with what it should have been all along. Sometimes you determine that this is simply what you have to do, but it should not be something that you simply do as a matter of course. The code may stink, but it's bought-and-paid-for.

Re: Maintenance vs. Programming style
by dwm042 (Priest) on Jan 29, 2008 at 19:53 UTC
    In maintenace mode, where I *do* own the code, work with it often, and the people behind me formatted in entirely irregular fashion, then Perl::Tidy is absolutely my friend. Otherwise, as moritz, dragonchild and others have said, least possible change to the code.
Re: Maintenance vs. Programming style
by ysth (Canon) on Jan 30, 2008 at 03:01 UTC
    I've inherited code written by someone who wrote verbosely. I keep finding myself saying "I could make this so much smaller, so much more efficient".
    I completely agree with you. And completely disagree with you. It really depends what you mean be "verbose" and "efficient". An example would have been very illustrative.

      Definitely true. “Smallness” and/or “verbosity” make not-the-slightest bit of difference to the Perl compiler, which is going to translate the whole thing into a p-code tree anyhow, and “efficiency” is pretty much subject to exactly the same argument-of-irrelevance. (At a billion ops-per-second, no one can hear you scream.)

      A good rule of thumb that I have adopted over the years, borne from the reality that for me it was true, is ... “would you spend your money to change this? If so, how will you convince your ever-skeptical board of directors (good folks... great at asking the hard questions) that you are actually right?”

      If the answer is “no, there really isn't a short-term Return On Investment (ROI) in this,” ... don't do it! Spend the money on a really good beer, instead.

Re: Maintenance vs. Programming style
by dsheroh (Monsignor) on Jan 29, 2008 at 21:01 UTC
    I tend to the "minimal change" end of the spectrum, because I generally work as a hired gun and have no idea whether I'll ever see the code in question again unless I'm working with an established client who I know will want me to continue to maintain the code for its lifetime - in which case I probably was the original author, so the style issue wouldn't arise in the first place.