On maintaining old code and the battle of styles...

Maintaining someone else's code is a laborious and tedious task. I believe that most of the monastery monks here had to go through a similar experience either at work or in the open source community. The job of adding or extracting anything from existing system/module is made even harder by a combination of either of these: poor documentation, inadequate design, or simply conflicting coding style.

For the purpose of this post, I'd like to focus on the coding style conflicts. Just recently I happened to help out with a few enhancements to a Perl module (on cpan). The module didn't have glaring design flaws or anything of such nature. Yet, I found my coding style and taste conflicting with that of the original author of the module (no longer maintaining it).

For example, my preferred way of passing arguments to a sub is using a single hash parameter instead of a lengthy list of arguments (this crosses over with the passing arguments node). Just like here:

sub _update_term_score {
    my ($self, %args) = @_;
    
    # %args = (
    #       term =>   search term
    #       doc_id => document id
    #       count =>  number of times term was found in this document.
    # )
    
    . . .
}
[download]

instead of,

sub _update_term_score {
    my ($self, $term, $doc_id, $count) = @_;
    
    #       term   ->   search term
    #       doc_id -> document id
    #       count  ->  number of times term was found in this document
+.
    
    . . .
}
[download]

Further, there are subtle conflicts such as not using single quotes for hash key values (that's what I prefer):

my %hash_var = (
    param1 => 'value 1',
    param2 => 'value 2',
    param3 => 'value 3',
);
[download]

instead of,

my %hash_var = (
    'param1' => 'value 1',
    'param2' => 'value 2',
    'param3' => 'value 3',
);
[download]

To continue, I'd have to write an extra 2 pages of similar rant and plea for you to read to the end (unlikely eh? :). Therefore, on I go to the actual conclusion and key question...

What really bothers me in all this is realizing that anyone coming to maintain this module after me will have to deal with both _my_ code and that of the original author. Mind me, there's nothing inherently wrong with either styles, it's all in taste and the attitude. So, would it be more considerate of me to simply follow the initial lead and try to stick to the existing code style as much as possible? Besides, if I were to look at a code with two or three or ... (you get the idea ;) conflicting styles, I'd assume (probably wrongly) that the code was designed poorly or that the author(s) were not competent in what they did... besides that's what every book on good Software Engineering would tell you.

"There is no system but GNU, and Linux is one of its kernels." -- Confession of Faith

Comment on On maintaining old code and the battle of styles... Select or Download Code

Replies are listed 'Best First'.
Re: On maintaining old code and the battle of styles... by chromatic (Archbishop) on Apr 25, 2002 at 14:42 UTC
It is good form to conform to the project's preferred style. On the other hand, having inherited the care and feeding of a few codebases, the first thing I do is to run perltidy to clean things up to the point that they don't offend my eyes.	[reply]
Re: On maintaining old code and the battle of styles... by Biker (Priest) on Apr 25, 2002 at 14:45 UTC
Do you intend to keep the ownership for 'the future'? -- Apply your own style whenever you make changes and/or additions. Potentially, work through the module applying your style all over. (You will learn a lot more about the module in the process.) Do you think you will soon hand this module over to someone else? -- Follow the same style as the original author. Do you think the module is mature enough to not need any more significant development? -- Consider following the same style as the original author. Everything went worng, just as foreseen.	[reply]
Re: Re: On maintaining old code and the battle of styles... by pdcawley (Hermit) on Apr 26, 2002 at 06:40 UTC
Please, if you do decide to change the style, then change it everywhere at once. Especially the parameter passing stuff, where consistency is really important. I've had to maintain code that was written in a mishmash of styles and it's just painful.	[reply]
Re: On maintaining old code and the battle of styles... by andreychek (Parson) on Apr 25, 2002 at 15:04 UTC
I think a good way that this is often dealt with is for the original author to create a written style guide for their particular application, and for each author afterwards to follow that guide. Unfortunatly, since that isn't defined by the original author, you're going to have to be the responsible one to take care of that :-) It may be hard enough for you to go through someone elses code, and learn enough about their styles and preferences to figure out what they are trying to do with their code. However, as you're suggesting -- if you change styles for code that you add, it's going to become very tough for anyone to read it (possibly including yourself after a few weeks of maintenance), without getting a headache :-) I love documentation. Perhaps what you can do is create a document, explaining the currently used style. Then, if there is an area where you feel a style could be improved (and it sounds like there's plenty of them), change both the code, and the documented style guide, at the same time. Yes, that can be a big deal, but it's probably going to be much easier to do that, than to maintain two seperate styles :-) I'm very fond of the Style Guide for P5EE, you could always use something like that as a basis. Good luck! -Eric	[reply]
Re: On maintaining old code and the battle of styles... by beernuts (Pilgrim) on Apr 25, 2002 at 14:46 UTC
Interesting question, vladb. I'm in a similar situation, as I develop code with 3-4 other people. What I've noticed is that when we write our own code, it tends to be in the style in which we're comfortable. When we collaborate, we work in a kind of group style(that none of us is really happy nor unhappy with). As you're not collaborating on this, I'd suggest you do it in the style that you're comfortable with - you're less likely to make mistakes, and you'll make progress more quickly. Just work hard to make sure you don't fall into your first or second traps: poor documentation/design, and all those who come to the module after you shouldn't have a problem. Besides, who's to say they won't like your coding style better? Would you really want to deny them an oasis in a desert of code? =-) -beernuts	[reply]
Change what you want internally, but not externally... by RMGir (Prior) on Apr 25, 2002 at 15:05 UTC
If you're going to keep ownership of the project, then by all means re-indent/adjust the internals to the style you're comfortable with. You probably can't change the calling convention from "multiple args" to "one hash for args" for any exported routines or methods without breaking the existing user base, though. -- Mike	[reply]
Re: On maintaining old code and the battle of styles... by Molt (Chaplain) on Apr 25, 2002 at 21:13 UTC
Have you thought of using something like Perltidy to reformat the code in accordance with your preferences? This way you have your favourite indenting style etc, and keep things consistant. I'm not sure this'll pick up things like the quotes round key values, but it should resolve most issues.	[reply]
Re: On maintaining old code and the battle of styles... by talexb (Chancellor) on Apr 26, 2002 at 14:01 UTC
Excellent thread. You have to choose whether you're just doing quick fixups, or you're taking over the module. For a quick fix-up, leave the coding style uniform -- that way there's only one style to complain about. If you're taking over the module (which seems more likely seeing as the original authour has abandoned it), then by all means rip it to shreds, then piece it back together again. Make the signal to noise ratio as high as possible. --t. alex "Nyahhh (munch, munch) What's up, Doc?" --Bugs Bunny	[reply]


good chemistry is complicated, and a little bit messy -LW
	PerlMonks