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?"
Unless he's making your own job a lot harder or you're his boss (or project manager), it's not your place. Your "help" will likely only piss him off more and more and cause problems in the office. Not only will it in *no way* benefit you, but it will very likely *hurt* you and your career--since your manager will come to view you as a source of headaches, your co-worker(s) will view you as a pretentious little prick, and (contrary to popular belief) the guy who helps produce better overall product is almost never rewarded for it anyway. About the fastest way for anyone to piss off their co-workers and bosses is to walk around with a "I'm the best coder here" attitude all day, whether it's true or not. Don't do it.
So, STFU and let management deal with him (or not). That's what they're paid for, not you. Don't offer *any* unsolicited criticism, and even if solicited, offer only a few minor criticisms at a time.
In short: Lighten up, Francis.
What political party do you join when you don't like Bible-thumpers *or* hippies?
There's a reason why he's a seasoned programmer and still has a job. Think about it. Who else can maintain that cluster fsck? No one, that's who.
If Bad Code did not exist it would be necessary for Good Coders to create it.
TL;DR: job security
People are very proud of their work, and do not take criticism well. One way is to ask for his advice on your work. In reviewing it, he may pick up some of your ideas and implement them. Be tactful.
Require that code be run through code analysis and compliance tools that look for the basic things, like function naming, function decomposition, pointer use and other pitfalls.
Then when code reviews come around, you have data instead of arguments and suggestion. "You've got 500 warnings from the code compliance tool. Clean them up before bringing the code to code review."
I should use this sig to advertise my book ISBN-13 : 978-1501515132.
That's what code reviews are for. They aren't to berate someone for writing bad code but identifying as a team areas for improvement (of the code, not the developer).
If you notice a lot of repetition, suggest refactoring common code into a function. Suggest renaming poorly named constructs. Code reviews should be a team effort with backing and consensus of the team with a focus on code quality and not any one developer's ability (or lack thereof).
Ascalante: Your bride is over 3,000 years old.
Kull: She told me she was 19!
Instead of criticizing him, say "I'm really struggling to read your code, could you do x y z to make it easier for me to understand? Thanks".
You'll find way to the explain it to him right next to the section with answers to questions from the wife, such as "does this make me look fat?"
Make code reviews mandatory for everyone. If he can't deal with that, he knows where the door is. On my team we use git, and contributors are not allowed to push their own code to the main branch. They must submit a pull request which gets reviewed and pulled by another member of the team.
You double-down. You take two-year old code that he's writted, and you ask him to walk you through it today. If he can, at reasonable speed with reasonable prep, you lose and you'd better start learning to read his code which conclusively isn't bad it's just different.
Seriously that is how I started writing good code. If it was code he hasn't looked at in years it works best as then he has to suffer through his own problems and wondering what the hell is going oin. I learned this very early on when I was still in school and was trying to create my own game on the side that I would work on periodically. After dealing with some of my own early garbage code (piss poor names, no documentation, shit structure) I learned real quick to make my own life easier and write better code. I know I write better code now than I did then and am always trying to be better.
Time to offend someone
That's nice in theory, but in practice, the "top priority" of code is to meet the deadline and get shipped. Everything after that is secondary.
I've worked in a few places which had some star coder who cranked out endless volumes of shitty, un-maintainable code. And as long as they kept churning out new features and the like, it was fine.
But god forbid that code should need to be maintained or updated -- the original author has moved onto other shiny things, can't remember whatever bit of genius led to the creation, and is often no help in debugging.
This is the kind of thing which is a long-term liability, but overlooked by people with short-term focus.
You can't fix him or his code. You can either cover your ass, or move on somewhere else. And somewhere else is just as likely to have someone of the same ilk.
If management can't/won't rein him in, he'll keep doing it. If he's really been coding longer than you've been alive, you stand no chance -- because either his code is brilliant and you just don't get it, or it's really crap but nobody cares because he can ship a new version on time.
It goes the other way too, I've known coders who were so focused on building something elegant, theoretically good, and built on the latest best practices their code was unusable. Those guys will never actually deliver anything which works, but they'll be able to give you a lengthy discourse on why this code convention is vastly superior to the one you've been using for the last 10 years. And, sometimes, those guys are also horrible at maintaining their code since they can't figure out how it does something any more.
And, since as a group developers tend to be high-strung, self-centered individuals who think they know everything (no slight intended, I used to be the same way) -- there's a reason why it's been compared to herding cats. It takes work to steer coders around, and it sounds like nobody is taking ownership of that where you work.
Lost at C:>. Found at C.
Since I'm arrogant like that guy and sometimes write less than beautiful code, I bet what works on me would work on him. He won't change radically overnight, but you can affect change without drama. .... . D you think I should break this function into two, maybe buildSpreadsheet() and saveSpreadSheet()?
Would that be better, it terms of "Functions should be short and sweet, and do just one thing", do you think?
Two days later you ask him to look at your variable names and tell you if he can tell what each is for, or if he thinks you should rename any of them to make it more clear. He's helping you to make sure your code is readable and clear.
Work with, not against, his personality. He thinks he's the smartest guy around. So go with that. He's your new mentor. A few times a week, ask him about YOUR code. Say something like:
Can you look at this function I'm writing, please? Linus Torvalds says that "Functions should be short and sweet, and do just one thing, and they should have more than 5-7 local variables." The Microsoft guidelines say
Ask him if you should make the thingRetriever a separate object from the stuffDisplay in your code. He'll likely eat it right up and never realize that he's actually getting a two minute lecture on a different aspect of code quality each day. If he starts out the day thinking about these things, he'll soon notice when he's writing garbage and his ego won't let him keep putting out garbage for long once he's aware of it.
If, after a month, you see zero improvement in his new code, then ask polite, direct questions about his code as needed. "I'm working on Foo(), what is the "bob" variable?" "Hey John, what does is this function called tp() supposed to do?" (My predecessor actually used "bob" as the name of at least one variable in each project.) Those questions make it explicitly clear that his code is unreadable, so do this only after you've kissed his butt in step #1 for best results.
Fire him.
Did you miss the descriptive word "coworker" in the parent post ---
or the even more telling phrase "a very smart person who has been programming since before I was born?
This is the new kid on the block.
He is not the boss.
As a guy in his mid forties this is very true. The first time you go to a doctor that's younger than you, or get pulled over by a cop that's younger than you and have to call him "sir", you feel it.
The thing is you have to recognize it and fight that impulse. Everyone over 21 is an adult. Remember that. Mozart began composing at the age of five. Einstein came up with some of his best work while he was a patent clerk. Not every good idea comes from someone your own age and station.
And it's especially important in the software industry. Younger guys have an advantage us beginning-to-fossilize programmers don't have. They're *flexible*. They can and will use the newest technology and do things we can't. If you don't listen to the younger folks in this field, you will eventually look like your parents did when they couldn't stop the VCR from blinking 12:00. That's how you'll look.
If a kid gave me a book on being a better programmer I'd read the fucking thing. Ego be damned.
Weaselmancer
rediculous.