Slashdot Mirror
Ask Slashdot: How Can I Explain To a Coworker That He Writes Bad Code?
An anonymous reader writes "I have a coworker who, despite being very smart, and even very knowledgeable about software, writes the most horrible code imaginable. Entire programs are stuffed into single functions, artificially stretched thanks to relentless repetition; variable and class names so uninformative as to make grown men weep; basic language features ignored, when they could make everything shorter and more readable; and OOP abuse so sick and twisted that it may be considered a war crime. Of course, being a very smart person who has been programming since before I was born makes him fairly impervious to criticism, so even a simple 'Do you see how much better this function is when written this way?' is hopeless. How can I make him see the light, realize the truth, and be able to tell good code from bad?"
conduct code reviews here is a good tool http://www.atlassian.com/software/crucible/overview , although it is not necessary to use a tool.
Run his code through a static analysis tool with default, or best practice, settings, and then have him explain why the detected problems are false positives.
The best way for people to learn that their code sucks is to have a 3rd, impartial, party tell them. Static code analysis tools are the best, because they have no ears.
Telling them directly their code sucks is the worst way.
That's a really cynical view. You must work at a really crummy place if that's how concern for long-term code quality is treated.
Where I work, this kind of behavior (helping others with their code quality, even those more senior than you) would be more likely to get you recognized/promoted.
Parent is pretty right. Been there, done that.
I *WAS* my job to try to introduce better practices, but since I was given no authority (i.e.: I can not fire anyone), I just got burnt.
People that was willing to learn better ways didn't need my criticizing, they just looked at my code and adopted the ideas they liked. Eventually almost all of that ideas come to production (and the few that didn't, oh hell, it appears that being a freaking paranoid doesn't necessary makes your code always better, uh? =P)
People that was not willing to learn nothing new just ignored me and, even worst, started to undermine me on the company. Things escalate to a level that I decided that the money wasn't paying the headache.
So, *DON'T DO IT* unless you have the power to lay off bad coders.
Lisias@Earth.SolarSystem.OrionArm.MilkyWay.Local.Virgo.Universe.org
I disagree.
Give the guy a book on the subject, explain in detail the difficulty in maintaining or interacting with his code.
Do it in a helpful and non confrontational way in a private setting.
I'm sure he can be convinced that making everyone else's job harder isn't beneficial to the company.
Maybe something like
Hey Bob, I wonder if we could break this monolithic program into four parts for easier maintenance....
Can we break this function out to a stand alone routine, because I call this function it loads in this ton of code that I don't need, and puts the database at risk...
Bob, I can't follow this code, what would you say to this re-write I sketched out...
Old dogs can be taught new tricks, but don't dismiss the possibility that the young pup doesn't quite know everything he thinks he does.
I've often seen kid programmers using some high level function without a clue about the mountain of code it drags into the
application, or use a language "feature" that generates a ton of code that is slow and imprecise, when discrete operations would
be much faster in execution and more accurate even at the expense of more source lines.
People working on the same project have to act as a team. Approached properly, the old geezer might actually appreciate it.
Sig Battery depleted. Reverting to safe mode.
I second the automation argument. Have an automation tool flag them as warnings. Then have a policy that code does not go out to build if the number of warning is over a set limit. Start the limit at the current number of warnings. Then lower the number gradually with each release. You can even get plugins for your IDE that display the problems as errors. Its hard to argue with a plugin that places a red x on your function.
...the book Code Complete
I used to be 29 when working with a colleague who was 64 at the time. He learned programming in ALGOL on Burroughs mainframes. Very tight, very sparse, very unreadable when he did Perl. Perl in fact lets you do this, as does C, but it's not needed anymore. It was downright impossible to get him to rename functions and variables to something descriptive or to use comments. That stuff used to cost back when the bits of RAM were visible with the naked eye. Today the balance tips another way. Doesn't mean the old dogs see any merit in that though and they will wield their seniority when pushed too hard. Personally I can really enjoy tweaking 6502 or 68K assembly by hand, but that's a hobby. At work I deal with business applications that, when slow, usually are just fed more iron because it's cheaper. The inner geek cringes at the thought, but it's the reality of business.
Learn from the mistakes of others. There isn't enough time to make them all yourself.