Maintaining the semantic content in the face of badly written code is a more tricky issue. When I am tasked with fixing buggy code that is badly written, I usually consider how much time I have available and the amount of code that needs to be changed. Refactoring ugly code is an almost palpable desire among good programmers, but that has to be balanced against time pressures and the introduction of new bugs in the refactored code.

If time is short and the change is a line or two, it is probably just best to make the change fit the surrounding code, along with a comment as to what was changed, and a corresponding test in the suite.

If there is more time available and/or the bug is more fundamental, I'll try to refactor the subroutine(s) that contain the bug. If this is a personal project with no time limit, I might refactor the whole program :)

No matter what the size of the change, it is good to add a test to check your work; larger refactorings may need multiple tests.

I'd check with the boss or owner of the code as well. I once joined a physics lab as a new grad student and the lab space was a complete disaster. While waiting for parts for my expt, I organized and cleaned up the place. My advisor took one look, and had a full-blown conniption: eyes popping, orange smoke coming out of his ears, the works. I had messed up his organization. We spent hours getting it back to it's original state.

I had to write a little blog application to integrate into our companies flagship site. All new work is done in PHP (that's a seperate rant), but prior to me being hired, they had one Perl programmer. Since she did all the initial work for the site, all the old code is in Perl.

For various reasons, mostly having to do with her maintaining the code, I was asked to write this little blog tool in Perl. Looking back over some of my code, I was appalled at how I had immitated much of the code that my program had to interact with. Globals everywhere, using a crappy internal library for parsing cookies, stuff I never do in fresh code. This was made worse by the fact that the senior Perl programmer doesn't write this way either anymore. Just the core (and therefore oldest) parts of the code are written that way.

I spent a week refactoring and rewriting the codebase, and created a list of things I would not fall into the habit of doing as I added new features. It was hard initially. For example, I need to require some definitions in from elsewhere, but this pisses strict and warnings off horribly. I had to wrap the include in some protective declarations, and then push the setup code into a subroutine to get the illegible mess out of my eyes. But being able to rely on compiler warnings has already saved my butt a few times since.

The key I think is to do what I did. Keep style imitation (and bug imitation) in old code. Don't be too subtle - when the original programmer does something dumb COMMENT IT! You'll be less likely to imitate it later, and more likely to find the subtle bugs that it creates. Keep a list of common mistakes and look it back over when transitioning between the two. Talk to the original programmers and see if their own style has changed enough to make it viable for you to correct some mistake in their old code.

Upon first reading this, I wondered how I would feel if someone found a mistake and surreptitiously fixed it without notifying me. Ninja coding seems to have a set of prior assumptions that I don't know if I agree with. Here's what comes to mind:

• I am truly fixing these people's buggy code and not inserting a sneaky bug into it.
• It's better to fix code behind the backs of my coworkers rather than talk to them about a possible error that I have found in their code.

Depending on your situation, this might be a reasonable thing to do. If you're the lead programmer on a time-critical project, this may even be a good thing (at least in the short term). But I believe that fostering a programming environment of creativity in which people can learn from each other is good for everyone's coding, so in the long term you may want to rethink your ninja ways. By practicing better communication, you may teach your peers a thing or two (and perhaps even learn something yourself). If you demonstrate best practices of coding to those around you their "bad" coding style may be vanquished by your "good" coding style in time.

It's interesting that you're being assimilated by the bad code that you are fixing. I would say that one key to learning good style is communication, by reading books, talking to people, and understanding the concepts behind various styles of coding. Perhaps if you communicate better you would be a Zen Master, rather than a Ninja. :)

With large projects working on each others code is essential. Programmers cant just drop their current project to fix a few minor thinkos in their code that are causing someone else problems. Programmers leave (and then become the instant cause of every bug and failure in the system :-) programmers drop projects, etc. While sometimes it rankles when it happens to your own code because you would have fixed it differently/better the fact is its almost inevitable with large code bases.

A good example is PM. The code base here has had many authors, we cant stand around waiting for those authors to show up to fix bugs or extend the code. If they are here and can do the work needed then fine, otherwise the work must get done.



---
demerphq


First they ignore you, then they laugh at you, then they fight you, then you win.
-- Gandhi

As an extreme example imagine that other author is you a year ago. Now you looking at your old code and what?.. I think "Ninja coding" is bad in that case. On the othe hand it can be a good thing if another's code is good but different (e.g. Abigail's style of '=>')

Depends. Not everyone is fond of Abigail's style either — it's quirky and occasionally fun, but it's not gospel just because of who he is. On numerous occasions, tye has bemoaned the fact that we can't make the fat comma DWIM better than it already does because people do all kinds of quirky stuff with it; I like his argument.

Other aspects of a coding style are largely a matter of personal preference. I have my reasons for the style I chose for myself, but it's suited to the way my perception and cognition work, and I doubt anyone would agree with all of my choices. The important part, in that case, is to achieve consensus among all parties and then be consistent with it.

Tools like PerlTidy are also helpful to keep negotiations to a minimum, as the code can be re-styled to meet the official style guide or any individual's tastes on the fly. This allows people to have their cakes and eat them too — they can disagree and still cooperate without having to negotiate a compromise.

Makeshifts last the longest.

I don't think that's a relevant example. The essential difference is that you, a year ago, will never come back, and will not edit the code anymore. The other author however, may.

You should code like a samurai. Then if you screw up your code, you can have some redemption in dying well (bravely and with dignity) in seppuku.

As far as I know, sepukku is not brave with dignity.
What you mean is probably harakiri.

Correct me if I'm wrong

You have it the other way around. You should google it.

When one opens an existing file written by a different party, puts in a fix that matches the style of the initial author with no traces left behind (except cvs knows :) )

I see good and bad with "Ninja Coding."

the good

the bad

When coding in a cooperative environment it is a good thing to maintain lines of communication. So if I change someone else's code I usually put comments in that explain that a) I did it, b) what I did and c) what my rationale was for doing it. Sometimes I've had to modify someone's code due to a bug report or a service ticket so I put the MR or ticket number in the comments as well as in the CVS log message.

Note: for those not exposed to Dilbert®, PHB means Pointy Haired Boss.

When you say that the only trace is through CVS, my assumption is that there was prior indication of a necessary code fix. Maybe it's just that the last place I worked had a whole process on when you want to check something out that's already been reviewed. First, you had to submit a problem report, and then a software lead would approve it and assign it (probably to the person who originated the report, but to someone else if load balancing was necessary). Then whoever is assigned the task gets to check out the file, make changes, check it back in, and submit it for re-review.

So what I was thinking was that once all that hoopla was accomplished, you go in and do your ninja coding. No style changes, trying to make your modifications as close to the original as possible.

However it seems that the majority of the other monks think it's about fixing some code issue without telling anyone. And I'm just not seeing that in the original post. But I admit I may be influenced by too many years of working in an environment that conforms to FAA guidelines.

Am I the only one who is reading it like this?

No. I did not see any indication of "fix without telling anyone" either. To me, the question was simply "is it a good or bad thing if I try to always follow the original style of the code so closely that my changes blend right in?"

Makeshifts last the longest.

yup. That would be it right one the button. I actually enjoy perfecting my ninja skills of modification without being noticed.

While doing so can be painful, I have actually experienced the pain of breaking this rule also. Breaking the rule is worse.

There are two outcomes of breaking the rule. The first and most obvious is everyone does and the resulting code is so abysmally horrendous it starts to rot before your eyes.

The second is a complete, elegent solution coming as close to beauty as you can imagine. You're co-workers will likely crucify you for it. The reasons for each will be different, the result will be the same. Your efforts will NOT be appreciated.

When dealing with old code that is no longer representative of current coding styles, changing the coding style should be an agreed upon project or it should be left as is. Mixing it up or fixing it unauthorized will never work in your favor.

Someone above mentioned the pain of reorganizing someone else's play pen. Believe it...

If you feel you have lost yourself, create a personal project outside of work for personal development. Your employer doesn't care about your being creativly restrained. Your employer wants you to be replaceable. Code only you can maintain restrains your manager. Code your co-workers have to be trained to understand costs your manager. Code that looks like all the rest makes your manager comfortable and is what they believe they are paying you for.

I have a better suggestion that I read elsewhere (maybe Joel on Software). Rather than stealth code fixes, write good test cases that expose the bug.

This has several advantages. First, you get to write good code even if it is just for a test case. Second, you force the fix to happen. Third, you force the bad coder away from new development and towards fixing their bugs. The original article I read suggested burying such coders in bug reports to prevent them from inflicting any more (new) damage on the system.

Hm. Instead of merely correcting the code, enlighten the other person through the use of a question that forces them to see the error.

The Zen Of Testing. I like. :)





-----------------------
You are what you think.

For some reason this comic springs to mind, although it probably shouldn't.

* Just kidding folks - really.

Like you say, not potentially the best code but it works. If you do it for enough different other people, I suspect you will learn others "styles" and hopefully over time learn the best from each style and eventually you'll develop a personal style that is pretty good.

I am a noob really, but hacking/Ninja coding others code has taught me a heck of a lot. But I too was not sure if I could actually code anything on my own or at least in my own style.
So... I started a sourceforge project and I am slowly writing my own code and getting better as I go along.

Ninja Coding is great for learning, but starting fresh with your own style takes it from learning to mastery. (well I hope it will anyway!)

Lance

My approach is that by not forcing myself to stick with the same style as the original developer, I can see (albeit not with a perfect success rate) who put what where. If the SQL query has uppercase keywords then I probably wrote it (or rewrote it).

More importantly, if something has stopped working and the query is all lowercase, one of the other guys could probably explain what has changed and why, rather than me trying to determine whether I am at fault. CVS doesn't get used much here (except in the stuff that is basically my exclusive area), so those factors will play less of a part for someone that has change logs etc.

My style rule: If you have to maintain the someone elses code style, your sanity level will approach the sanity level of the original developer.

I've worked on a large code base with many developers in the past. In general it is a good idea to keep the existing author's style especially if they are still maintaining the code, and it doesn't drive you crazy. I don't look at it as losing myself, but rather being able to adapt.

That said, I feel your pain. My sanity took a hit a while back when I had to maintain a fair amount of code that someone reformatted to their "taste" since they didn't want to maintain the original style. This taste included making lines go past 100-columns by using crazy indentation rules and lots of levels of indentation, introducing non-English comments and variable names to code written entirely in English, and using huge variable names (>60 characters long in some cases). This made it impossible to even diff the code against previous releases and determine what changes were actually made, even with a version control system. My choice was to keep with their "style", reformat the huge amount of code again, or find other work. The other work is working out just fine...