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?"

Comments

You don't

[crazyjj]

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.

Re:You don't

[Anonymous Coward]

I disagree. You do this through code review by linking to your team/company coding standards (you do have those, right?).

Also, are you sure that his code is that terrible? It's always possible you just don't understand his way of thinking. Maybe ask him why he wrote something a certain way and go from there.

Re:You don't

[coma_bug]

"I'm the best coder here"

no, but "you're the best coder here but it would help if you wrote code the rest of us can understand" might work.

Re:You don't

[lorenlal]

I completely agree with this. Hyperbole aside, if the coding practices in your organization allow for this, you should discuss them (and not the practices of others) with your management. If the coding practices discourage (or even forbid) this behavior, and your coworker continues to violate them... That's not your problem.

Iff his code does affect what you are working on in a negative way AND it violates coding practices that your organization is enforcing, you could bring it up to immediate superiors.

Re:You don't

[idontgno]

Or not. More than once, the reponse I've seen was the icy equivalent of "Your understanding is not my problem."

Seriously. If there's no violation of actual standards, browbeating someone about coding style is both futile and counterproductive. Even if it's horrible coding style. Without the firm ground of enforceable standards, it's just an unwinnable piss-fest.

As glurgy and trite as it comes across, sometimes the Serenity Prayer is the best answer. Especially the part about wisdom.

Re:You don't

[Darinbob]

If other programmers can not understand it, then it's bad. Even very intelligently designed code can be awful if no one else can understand it. If you're working in a team then you absolutely must make sure your code is not a burden to your coworkers. Sometimes it's just a matter of putting in comments to explain what you're doing.

Re:You don't

[BitZtream]

And that shows that you're a shitty programmer.

If you're gut reaction to criticism, constructive or otherwise, is to behave like a 12 year old, you are a shitty programmer.

I can say this because my biggest flaw as a developer is responding poorly to critical input from others. I've at least over the years got to the point where my initial reaction may be shitty but I do sit back later and analyze the input to look for truth in it that I may not realize.

I've learned more from CS graduates who have been out of school for less than a year than old foggies like myself. Not because they are über programmers, but because they had a perspective that I could not consider myself, even if their perspective was one of ignorance it almost always helps me be a better developer when I learn how to adapt to their complaints.

This is a process that never ends imho. Writing code to get from point A to point B is brain dead simple. The path you take to get there is what matters.

Re:You don't

[Austerity Empowers]

if his attitude is like some of my coworkers, he's no doubt asking his management why the junior engineers aren't rewriting his code to make it efficient while he tackles the big problems more suitable to his seniority.

Re:You don't

[funwithBSD]

Help him get promoted to "Software Architect" and get him out of the coding business.

Re:You don't

[Nefarious Wheel]

Sometimes, subtlety is overrated.

"John, that piece of code smells worse than your uncle's codpiece (which I wish you would stop wearing to work), and in reviewing it I can deduce that your parents were never formally introduced."

Re:You don't

[Garridan]

If other programmers can not understand it, then it's bad.

I can't understand most of the kernel source. My C skills are rusty, and I know nothing of kernel design. That means it's my fault, not the kernel hackers'. Is it possible that the submitter isn't seeing the whole picture, and failing to take his own ignorance into account?

Re:You don't

[Beetjebrak]

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.

Re:You don't

[Altus]

I'm going to assume that the poster is not an idiot and that this guy really is writing horrible code as described.

What I am mostly hearing here is that the company lacks things like basic code reviews or any kind standards. I cannot imagine how this guys boss is ok with checkins like what the poster is describing. This sort of thing would be caught at any half way decent company.

Sure, this guy sounds bad but it has to be symptomatic of the company having serious issues. Its probably time to find a better job while you still have this one and perhaps at the exit interview you can clue some people into some of these problems.

Or just suck it up and deal, but this likely wont be the last problem.

Re:You don't

[dubbreak]

What I am mostly hearing here is that the company lacks things like basic code reviews or any kind standards. I cannot imagine how this guys boss is ok with checkins like what the poster is describing. This sort of thing would be caught at any half way decent company.

Not necessarily. I worked with someone that directly fit this description. I've never seen OOP abused so badly in my life (inheritance and polymorphism that was either the product of a genius obfuscating their code.. or an idiot that knows of such ideas, insists on using them, but has no clue as to the right time and place to use them). He was able to sneak by most code reviews because he was often working solo on products that were borderline legacy. With few bugs apparent at the user level his slow progress was put up with for the mostly "reliable" code. Then once some other people got pulled (such as myself) he got put under a microscope by his manager. This actually resulted in yelling matches. The code was truly horrible. At one point he had read that a function/method shouldn't have more than a few lines code. I found a method 9 calls deep, each method adding another argument that resulted in a call to a native API that could have been done at the top level (well excepting side effects along the way that did god knows what). We also coined the term "lilypadding" for some of his solutions. It was obvious he's jump directly into a problem without any real vision of the end goal or result. He'd eventually get to the end, but only after making many many jumps, often in the wrong direction (open file, read all of file, find line you need, read part of the file based on that line again despite having just read the entire file, do select * on a table in a loop while filtering in code, etc). It almost seemed he came upon a working solution by happenstance. I think part of it was the false ideal: if you aren't coding you aren't working. A 5 day solution for him would be 2 days planning and 1 coding for anyone else.

In the end his code was not maintainable or extensible. He refused to listen to the suggestions of his manager and change his code, insisting it was golden and the correct way to program (despite being provided plenty of evidence to the contrary). The guy would even get pompous when you request he fix his own check-ins (he would often leave his hacked up "testing" library in references, though his special proprietary library was not in the codebase, resulting in a project that would not build).

How'd management fix the issue in the end? They got rid of him. He got decent severance, and help seeking a new position. Part of me feels for the guy (and his family that relied on his wage), but part of me also wonders who's problem he is now.

How was it solved? They got rid of him.

Could he have been trained to be better? Maybe, but he didn't act willing to change and wasn't a good fit to the team or company attitude-wise. He'd do things like hang up on a customer because "it was his lunch time" even after being reprimanded for doing that previously. He claimed in "his culture" that's how they do things. When a bug is escalated directly to the responsible programmer from a customer, you put your lunch off for 15 minutes, you don't say, "Oh it's lunch.. bye." And we're not talking a slave driving culture, he was free to take his lunch whenever and any overtime would be balanced by time off. It was simply a matter of treating the customer well in a sensitive situation which he couldn't wrap his thick skull around.

If a company isn't willing to take care of time wasting employees, that don't fit the company culture.. I'd leave. It's not your problem to take care. It's your job to make sure the issues are clear to management, and it's fair for them to request you help guide him/her to being more productive in the company, but it's not your job nor place to try to fix it.

Re:You don't

[TapeCutter]

Ever been in the situation where a co-worker asks you what a bit of code does, you skim it for a few minutes and brush him off with "not sure", then he replies; "but you wrote it"? If you write enough code (especially maintenance code), it will happen to you eventually.

So IMHO not really your "fault" that you can't pick up the kernel design in an afternoon by skimming the code, it's the coder's job to clearly explain the implementation, not the design.To paraphrase an old friend of mine, "Source code is like shit, you can't smell your own". Even if the code was written by Shakespeare, browsing code is still the hard way to pick up the design of a non-trivial application, but if your not surrounded by co-workers who wrote it then it's often the only way available.

Trivia: I used to teach C to second year engineering and CS students in the early 90's. The engineers were the worst for writing the entire thing in main() because they already had leaned bad habits from elsewhere. Each year most of them would fail the first assignment and then complain about the harshness of the assessment, one they knew I was serious about my style sheet most of them would do much better on the second and subsequent assignments. I wonder how many of them now understand why their code's comments and style were worth a full 50% of their mark.

Re:You don't

[Nefarious Wheel]

Simplicity and clarity is the mark of an expert.

Re:You don't

[WaywardGeek]

Great story. I've never had trouble understanding code I wrote. That's probably because I have one of the world's worst memories, so I always write code with lots of clues to help me decipher what's going on. Also, I got lucky when a programmer from industry taught a structured programming course. He was all about coding best practices. I learned the KISS rule from him, and have avoided clever obtuse code, when a straight forward solution will do. The analog designers I work with might disagree... I'm forcing us to use Python and classes, and you'd think I was murdering kittens from all the complaining. The reaction when I made us all use git... *shudder*

As for the poster's question, feel free to make some noise about his code, and see if it has any impact. Older coders like me rarely change much, but you might get him to adhere to a few team-wide coding guidelines, at least if he's not an analog designer. In a perfect world, you could help him find a better job, one where his coding issues would not be such a liability, and where whatever his real strengths are can come into play.

The best programmer, by far, I ever worked with is Ken McElvain, founder of Synplicity. If you need a problem solved quickly, and better than what all your competitors can do, he's your man. However, his code when I joined the company had some of the issues the poster mentioned, and that made me laugh. He's a very fast typist, but his typing speed is his limiting factor in coding, so he used 1 and 2 letter variables mostly. Writing code inline is faster to type than factoring out a bunch of small functions. Comments just slow him down. The first day working for him, I opened one of his most ingenious pieces of code (responsible for better 2-level logic optimization than what Synopsis had at the time), and tried to understand the 2,000 line file, which is tiny given the job it does. It's core is a brilliant 1,000 line function, and it took me all day to figure it out. I thought there had to be documentation somewhere, so I grep-ed the whole code base, all 350,000 lines of it, for comments. There was one. It said, "This is a hack." Many programmers, including me, will hold that his code is difficult to understand against him in the "best programmer" contest. He wins anyway. He knew that the most important reader of his code was himself, and that being as productive as possible early on was key for Synplicity's success. We were profitable, and had over 20 employees when I joined, yet I was the first programmer he hired. As the coding team grew, he started adding comments and using descriptive names. I heard a lot of complaints about working with some of his code, but he owned nearly 50% of Synplicity at the IPO, and everyone involved made money. Go ask his investors what they think of his coding ability. Anyway, if this co-worker is so smart and experienced, I'm not convinced hes a "bad" programmer. Maybe he just needs a job where he's the right guy.

Re:You don't

[rtb61]

Sounds like the problem can more readily be resolved by two levels of coders. The creators who produce the original lot and make sure it runs and then editors who tidy and pretty it all up and make in more informative. In fact lots of people who are not very successful at creating new code, can be quite good editors. Cleaning up code, resolving the last few bugs, better variable and class names, more comments, breaking up functions etc. They do it for writers in order to speed publication. The same might be considered suitable for coding and with ready access to the original coder to provide explanation over any more convoluted high quality easy to maintain code could be more readily and quickly produced.

Re:You don't

[starfishsystems]

Late in my career, I find myself surrounded by genius developers. That's rare. Mostly the better developers I have met are fast and smart, but somewhat conceited and not particularly careful. I run across someone with real talent maybe once every five years, if I'm lucky. And by talent I mean that they write consistently beautiful, correct, modular, extensible, exemplary code. Okay, maybe once every ten years would be more accurate.

The people I'm working with now, well, I'm still making up my mind. They really are geniuses, by which I mean that they're exceedingly smart, blindingly fast, and exceptionally careful in their work. It's hard to fault them, though it's not easy to understand them. And, apart from feeling totally outclassed, mostly I'm in heaven.

There's just one missing piece, and you touch on it in your comment. The best possible code is the most maintainable code. If it takes a genius to maintain it, then you need a reliable supply of geniuses. These, by definition, are in short supply. If the code is unpleasant as well as difficult to maintain, you will have difficulty motivating and retaining those geniuses. All of this serves only to magnify business risk. You would be far better off with genius code that anyone could maintain. But that takes real talent.

Re:You don't

[omfgnosis]

I don't even understand why you describe JavaScript as "easy to use". Easy to start using, perhaps. It's probably the same reason PHP is so widely used. Of course wide use means lots of terrible code, most programmers aren't particularly excellent at programming.

JavaScript is highly misunderstood by many programmers, and widely used by people we should properly call non-programmers. Not because of its "ease of use" but because of its wide availability and its position as the front-end web language. Additionally because of widely available, widely used libraries like jQuery that make it simple to do a few things that are simple for non-programmers to reason with. That's not a failing of the language, it's a failing of happenstance.

I've seen some incredible code written in JavaScript. I dare say I've even written some good JS myself. I'm not an idiot, and I quite like the language. There are things I might like to change, but they're few compared to the parts of the language I like. I like it so much, in fact, that I've chosen NodeJS for the biggest project I've ever worked on. It's the best tool for the job, in this case.

But you know... people love to hate JS. I don't want to convince you not to, I really don't care. I don't particularly like being called an idiot as a matter of guilt by association, just as I'm sure you wouldn't like it if I said:

It's possible to write insightful comments on the Internet. Nobody does though, it's just too easy to write crap that works well enough for now. It seems to be the great failing of any "easy to publish" network. Create something simple enough that even an idiot can post their facile opinions on it, and only an idiot will want to.

And I am not saying that, except as a tongue-in-cheek reminder that we all live in a glass house and we might be wise not to throw stones.

Ok ok

[Anonymous Coward]

I GET IT!!! My code sucks. You have made this clear. You don't have to start posting on forums you know I read. Sheesh....

He knows something you don't.

[VortexCortex]

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

Criticism should be handled delicately...

[Anonymous Coward]

Leave him an anonymous poem:

Roses are red,
Violets are blue;
The C obfuscation contest produces bad code,
And so do you.

code reviews

[k0ldsh4d0wz]

conduct code reviews here is a good tool http://www.atlassian.com/software/crucible/overview , although it is not necessary to use a tool.

Automate!

[TechyImmigrant]

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."

Code reviews

[Java Pimp]

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).

Provide and solicit feedback.

[Tackhead]

A good manager should provide and solicit feedback.

For example, you tell him his code code not functional or elegant, and then, you ask him what he thinks about that.

And then you write the goddamn login page yourself.

mandatory code reviews

[alx]

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.

Old code

[holophrastic]

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.

Re:You write code for humans...

[gstoddart]

Since programmers must maintain code, read it, understand it, and write more than work with the existing code, the top priority of code is to be well written, and easy to understand for humans.

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.

Look at Linus

[rastos1]

Didn't Linus set an example just a few days back?

Buy him a present

[rabenja]

...the book Code Complete

That's true

[Weaselmancer]

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.