views:

715

answers:

20

If you take over a project from someone to do simple updates do you follow their naming convention? I just received a project where the previous programmer used Hungarian Notation everywhere. Our core product has a naming standard, but we've had a lot of people do custom reporting over the years and do whatever they felt like.

I do not have time to change all of the variable names already in the code though.

I'm inclined for readablity just to continue with their naming convention.

+24  A: 

Yes, I do. It makes it easier to follow by the people who inherit it after you. I do try and clean up the code a little to make it more readable if it's really difficult to understand.

Kevin
This is recommended in "The Practice of Programming" by Kernighan and Pike as well, for the same reason - ongoing clarity and maintainability.
Harper Shelby
And it is also recommended by "common sense" ;-).
Gamecat
What's funny is that there is an extremely lack of that "common sense" thing in our profession :)
Kevin
+3  A: 

Often, making a wholesale change to a codebase just to conform with the style guide is just a way to introduce new bugs with little added value.

This means that either you should:

  1. Update the code you're working on to conform to the guideline as you work on it.

  2. Use the conventions in the code to aide future maintenance efforts.

I'd recommend 2., but Hungarian Notation makes my eyes bleed :p.

Aaron Maenpaa
+2  A: 

Generally, yes, I'd go for convention and readability over standards in this scenario. No one likes that answer, but it's the right thing to do to keep the code maintainable long-term.

When a good programmer's reading code, he should be able to parse the variable names and keep track of several in his head -- as long as their consistent, at least within the source file. But if you break that consistency, it will likely force the programmer reading the code to suffer some cognitive dissonance, which would then make it a bit harder to keep track of. It's not a killer -- good programmers will get through it, but they'll curse your name and probably post you on TheDailyWTF.

John Rudy
+3  A: 

If you are maintaining code that others wrote and that other people are going to maintain after you, you owe it to everybody involved not to make gratuitous changes. When they go into the source code control system to see what you changed, they should see what was necessary to fix the problem you were working on, and not a million diffs because you did a bunch of global searches and replaces or reformatted the code to fit your favourite brace matching convention.

Of course, if the original code really sucks, all bets are off.

Paul Tomblin
+2  A: 

I certainly would continue to use the same naming convention, as it'll keep the code consistent (even if it is consistently ugly) and more readable than mixing variable naming conventions. Human brains seem to be rather good at pattern recognition and you don't really want to throw the brain a curveball by gratuitously breaking said pattern.

That said, I'm anything but a few of Hungarian Notation but if that's what you've got to work with...

Timo Geusch
+1  A: 

Personally whenever I take over a project that has a different variable naming scheme I tend to keep the same scheme that was being used by the previous programmer. The only thing I do different is for any new variables I add, I put an underscore before the variable name. This way I can quickly see my variables and my code without having to go into the source history and comparing versions. But when it comes to me inheriting simply unreadable code or comments I will usually go through them and clean them up as best I can without re-writing the whole thing (It has come to that). Organization is key to having extensible code!

John Chuckran
+1  A: 

if I can read the code, I (try) to take the same conventions if it's not readable anyway I need to refactor and thus changing it (depending on what its like) considerable

Calamitous
good test,to be honest some code was rushed,and the original author just did some ductaping, in these situations refacetoring is vital
Robert Gould
+1  A: 

Depends. If I'm building a new app and stealing the code from a legacy app with crap variable naming, I'll refactor once I get it into my app.

John Dunagan
+5  A: 

If you're not changing all the existing code to your standard, then I'd say stick with the original conventions as long as you're changing those files. Mixing two styles of code in the same file is destroying any benefit that a consistent code style would have, and the next guy would have to constantly aks himself "who wrote this funtion, what's it going to be called - FooBar() or fooBar()?"

This kind of thing gets even trickier when you're importing 3rd party libraries - you don't want to rewrite them, but their code style might not match yours. So in the end, you'll end up with several different naming conventions, and it's best to draw clear lines between "our code" and "their code".

Enno
+2  A: 

If the file or project is already written using a consistent style then you should try to follow that style, even if it conflicts/contradicts your existing style. One of the main goals of a code style is consistency, so if you introduce a different style in to code that is already consistent (within itself) you loose that consistency.

If the code is poorly written and requires some level of cleanup in order to understand it then cleaning up the style becomes a more relevant option, but you should only do so if absolutely necessary (especially if there are no unit tests) as you run the possiblity of introducing unexpected breaking changes.

Scott Dorman
+2  A: 

Absolutely, yes. The one case where I don't believe it's preferable to follow the original programmer's naming convention is when the original programmer (or subsequent devs who've modified the code since then) failed to follow any consistent naming convention.

Greg D
+1  A: 

Yes.. There is litte that's more frustrating then walking into an application that has two drasticly different styles. One project I reciently worked on had two different ways of manipulating files, two different ways to implement screens, two different fundimental structures. The second coder even went so far as to make the new features part of a dll that gets called from the main code. Maintence was nightmarish and I had to learn both paradigms and hope when I was in one section I was working with the right one.

baash05
+1  A: 

When in Rome do as the Romans do.

(Except for index variables names, e.g. "iArrayIndex++". Stop condoning that idiocy.)

Leonardo Herrera
what if the Romans have index variables like "rrrrrrrrrrrrrrr++"?
wonderchook
Did you know the Romans didn't have a zero, and so therefore had no way to terminate their strings? :-)
Paul Tomblin
Index variable names aren't subject to the "Romans" policy. Ergo, you can change that to "r++." Unless it's a global variable; in that case you are doomed anyways.
Leonardo Herrera
Tomblin, that's why you have Pascal strings.
Leonardo Herrera
paul, Joel has a great blog about strings ending in null. Also how did the romans use an array if they didn't have zero??? I don't get it.. haven't arrayes always started at zero :)That said I had a coworker who switched zero based and 1 based depending on the method that accessed the object. Ugg!
baash05
+1  A: 

I think of making a bug fix as a surgical procedure. Get in, disturb as little as possible, fix it, get out, leave as little trace of your being there as possible.

Metro
+5  A: 

I agree suggest that leaving the code as the author wrote it is fine as long as that code is internally consistent. If the code is difficult to follow because of inconsistency, you have a responsibility to the future maintainer (probably you) to make it clearer.

If you spend 40 hours figuring out what a function does because it uses poorly named variables, etc., you should refactor/rename for clarity/add commentary/do whatever is appropriate for the situation.

That said, if the only issue is that the mostly consistent style that the author used is different from the company standard or what you're used to, I think you're wasting your time renaming everything. Also, you may loose a source of expertise if the original author is still available for questions because he won't recognize the code anymore.

Sam Hoice
+1, I grew up a boyscout and they taught us to leave the places we go to in better condition than when we got there.
Robert Gould
+1 to Sam for common sense over blindly following rules. +5 to Robert for recognizing that the Golden Rule applies to software, too. :-)
Adam Liss
+1  A: 

I do, but unfortunately, there where several developers before me that did not live to this rule, so I have several naming conventions to choose from.

But sometimes we get the time to set things straight so in the end, it will be nice and clean.

Gamecat
+1  A: 

If the code already has a consistent style, including naming, I try to follow it. If previous programmers were not consistent, then I feel free to apply the company standard, or my personal standards if there is not any company standard.

In either case I try to mark the changes I have made by framing them with comments. I know with todays CVS systems this is often not done, but I still prefer to do it.

Jim C
+1  A: 

Unfortunately, most of the time the answer is yes. Most of the time, the code does not follow good conventions so it's hard to follow the precedent. But for readability, it's sometimes necessary to go with the flow.

However, if it's a small enough of an application that I can refactor a lot of the existing code to "smell" better, then I'll do so. Or, if this is part of a larger re-write, I'll also begin coding with the current coding standards. But this is not usually the case.

Chris Conway
+2  A: 

Yes. I actually wrote this up in a standards doc. I created at my current company:

Existing code supersedes all other standards and practices (whether they are industry-wide standards or those found in this document). In most cases, you should chameleon your code to match the existing code in the same files, for two reasons:

  1. To avoid having multiple, distinct styles/patterns within a single module/file (which contradict the purpose of standards and hamper maintainability).
  2. The effort of refactoring existing code is prone to being unnecessarily more costly (time-consuming and conducive to introduction of new bugs).
Kon
I did this too - Rule 1 "You MUST maintain the existing codebase keeping the same style, conventions and other formatting. This takes precedence over all other rules". Rules 2+... the usual stuff. Its perfect for having coding standards and then allowing common sense to prevail.
gbjbaanb
A: 

If there's a standard in the existing app, I think it's best to follow it. If there is no standard (tabs and spaces mixed, braces everywhere... oh the horror), then I do what I feel is best and generally run the existing code through a formatting tool (like Vim). I'll always keep the capitalization style, etc of the existing code if there is a coherent style.

My one exception to this rule is that I will not use hungarian notation unless someone has a gun to my head. I won't take the time to rename existing stuff, but anything I add new isn't going to have any hungarian warts on it.

rmeador