Dealing with co-workers who do not have a consistent coding style?

Question

What do you when you're working with someone who tends to write stylistically bad code? The code I'm talking about is usually technically correct, reasonably structured, and may even be algorithmically elegant, but it just looks ugly. We've got:

• Mixture of different naming conventions and titles (underscore_style and camelCase and UpperCamel and CAPS all applied more or less at random to different variables in the same function)
• Bizarre and inconsistent spacing, e.g. Functioncall (arg1 ,arg2,arg3 );
• Lots of misspelled words in comments and variable names

We have a good code review system where I work, so we do get to look over and fix the worst stuff. However, it feels really petty to send a code review that consists of 50 lines of "Add a space here. Spell 'itarator' correctly. Change this capitalization. etc."

How would you encourage this person to be more careful and consistent with these kinds of details?

Answer 1

Agree on a coding convention

Even if this is a one pager. I suggest that the whole team sit down and all agree on basic working coding convention that the whole team can use.

Answer 2

I think you just have to keep doing what you are doing. Have a clear set of coding guidelines, and enforce them during code reviews. If a developer gets 50 or 100 lines of "Add a space here" and "Spell 'iterator' correctly" every time he tries to check something in, and he is actually not allowed to check in before all of those get fixed, eventually he'll have to start writing cleaner code just to avoid the hassle.

I think if you fix these things yourself, like NimChimpsky suggested, you will be cleaning up after this person forever.

Answer 3

I call BS on everyone who said that variable name spelling errors and formatting don't matter. Obviously, they've only read their own code. And notice that word right there - read. Imagine reading a book with lots of spelling mistakes, mish-mashed formatting, inconsistent line spacing, and various other laziness prevalent in a lot of source code. It would be tedious.

For a profession where your syntax must be 100% correct to work, there is simply no excuse for any real developer to not have a clean, consistent code style. Anything else is sloppiness and laziness. I always question sloppily formatted code's correctness in implementation.

Answer 4

We have a good code review system where I work, so we do get to look over and fix the worst stuff. However, it feels really petty to send a code review that consists of 50 lines of "Add a space here. Spell 'itarator' correctly. Change this capitalization. etc."

I'd change it myself and then add a polite comment in the code.

this assumes that there is already a style guide as the question stated :

We have a good code review system

So my suggestion is a last resort, I figure its just as quick to change it yourself and leave a comment, as it is to send an e-mail or whatever.

Answer 5

I think conventions such as naming of classes and variables are important and should be followed through, elegant and efficient code too, but at the risk of having my answer downvoted many times, I have to say that in general the "pretty code" paradigm that is pushed a lot around is in IMHO very overrated.

First off, the developer who wrote it will have to maintain it in the first place, and if he is ever hit by a bus and another programmer cannot figure out how it works because the code is not "pretty", I'd say that the other developer is not very good anyway. And there is a lot of automated formatters/beautifiers out there, so anyone can use those to beautify the code if necessary, w/o wasting time while being "in the flow"/"in the zone".

Please note that I'm not advocating spaghetti/cowboy style coding here, in fact I've seen some very nicely formatted spaghetti code (function bodies spanning 4-5 screens, global variables scattered around different source code files, generally bad picks of names, etc).

Answer 6

One of my colleagues writes html in such a way that it makes my skin crawl. Imagine my html nice and structured with two space indents, hacked to pieces by tags added to the end of mine which end on the same line or on the next like some drunk who needs to throw his arm around you to stay standing. New lines are rarely indented but if they are, I'm sure there's some highly chaotic black hole in some part of the galaxy spitting out irrational temperature values in such a way that somehow its digits mirror the number of spaces or tabs used in such indentation by this woman. If I'm lucky, I'll see an input tag that's closed with "</input>". Total nightmare you can understand.

Nobody seems to understand this either, seeing how for most higher ups here, organized code or unorganized code to them is like the difference between us putting swiss cheese or american cheese on our sandwiches, which is to say, they could really care less. I started letting it slide because I was stressed with another project, and I think she began to realize how difficult it was to understand code like that before she wanted to improve. My advice would be to demonstrate why it's preferable to style your code more than simply telling them to do it.

Answer 7

The code I'm talking about is usually technically correct, reasonably structured, and may even be algorithmically elegant...

Be happy that you got all of that. Most programmers will maybe give you the first thing on that list. I think that variable naming and spacing is the least important thing to worry about.

Answer 8

Sounds like you need to set up and agree to a style convention. If you don't you'll have libraries that have 3 space indents, others that have 4, some that use Camel Case and other that use underscore_case.

Answer 9

Are the changes you want to make your personal preferences or do you have an actual standard to follow? If you don't have an actual standard, then don't do this. First set a standard. Then you can get software that can be set to refactor the code to the standadrd settings (well at least of some things).

If you have a standard, start enforcing it in code review. There is no point to having a standard if you don't enforce it in code review. This will mean a lot of extra work in maintenance as people will have to fix old code that didn't meet the standard orginally when they touch it.

Even without a standard, insist on fixing mispellings in variable names (I wouldn't particularly worry about comments) as they will drive everyone who touches the code crazy forever.

Answer 10

Coding standards need to be identified so everyone knows what they are and then they need to be enforced. There should be consequences to not following the rules.

Here are the things that should provide some incentive:

1. Code reviews will be tedious and longer than necessary.
2. Code will be rejected more often.
3. Schedules will not be met.

If this person either doesn't have to worry about this because no one enforces your rules or they don't care if they are unproductive (and no one does anything about that), there's not much you can do about it.

Answer 11

I'd be tempted to suggest having a private chat and see if both of you could find a root cause:

1. Is the co-worker rushed and because someone wanted the code yesterday the person is trying to get something working as fast as they can? This can be an opportunity to inform this person to focus more on quality than speed in their work. A mantra like, "Take your time," could be useful if this isn't counter-productive.

2. How does the person view their work? If there is a sense of pride, then you may have an angle to use in getting one to improve. If it is just a job that pays the bills, it may be a lot harder to get changes. Do they know they aren't doing a great job but are so close to it?

3. Does this person disagree with the conventions and is trying to code in protest? If so, then you may have a big problem, but this is worth finding out if this is the case or is the person merely lazy? What kinds of motivation may be useful here,e.g. could you appeal to greed, pride, or some other vice to get the person to improve. This is sneaky but possibly effective if trying the nice guy route gets one nowhere.

4. How to Win Friends and Influence People has a few suggestions in terms of being persuasive that may work, such as praising improvements and giving the person a fine reputation to uphold.

As for why this has to be done privately, here are a few reasons:

1. There is a good chance for humiliation, criticism or other unpleasantness that is better kept behind a door than left out in the open where someone may feel their character is being assassinated.

2. You want to encourage this other person to open up a bit. A challenge here is that some people are so guarded it can take a long time to get them to take their walls down.

3. If possible, I'd suggest trying to do this a little away from the office. Go out for lunch, take a walk, or do something so that the surroundings are altered enough that the person may feel a bit more comfortable. This can be a challenge and requires knowing the person, but the idea here is that in the office some people will wear a work mask that isn't likely to be helpful here.

4. Be prepared for the conversation to get rather heated or ugly, but this could well be a good sign if you can keep the other person engaged and have a good dialogue. Some people like to keep things out in the open and others may prefer more subtle ways of getting things done. The key is to make sure you are listening to the other person enough to empathize and try to understand their side.

Answer 12

We have a JUnit test that looks for formatting issues. It runs as part of the build. I continually get bit by omitting a space between if, while or for and the opening parenthesis. Our code is consistently formatted, though.

http://code.google.com/p/kawala/wiki/BadCodeSnippetsRunner

Answer 13

Code beautification like unscrutify will be able to solve some of your problems. If you are ready to pay for this then there are high level softwares which embed the rules in the source code itself like Parasoft. Parasoft makes it compulsory to write the code in uniform style. You can embed your own rules too. When such tools are used the developers are forced to use uniform style. And after a while they will get used to it.

Answer 14

If you use Eclipse, then enable Save Actions for the Java Editors, and tell everybody to use it. This fixes formatting issues on every save, but does not fix bad capitalization. It might be quite helpful though!

Answer 15

How hard is it to follow style conventions? I understand the spelling errors but the rest is an indicator of sloppy thinking and coding. Tell the person that they need to be more consistent when it comes to production code because they are not the only ones that will be looking at it. It's just plain rude, selfish, and inconsiderate to write production code in an inconsistent style.

Answer 16

LOL. You would absolutely hate my code. I can't spell to save my life and I don't care.

But I know some people do actually care about those things.

I suggest you fire the person writing that ugly code if they don't change then find somebody that makes things really pretty and hope they can write code that

is usually technically correct, reasonably structured, and may even be algorithmically elegant

and if they can't then at least you can show the broken pretty code to the customer and sell them on that!

But seriously. Focus on the actually important things first. If you can't find a good, solid reason outside "it hurts my delicate sensibilities" then ignore it for now. If it is actually important then sit down with the person and convince them of that importance. Things like standards that make it easy to tell the difference between class level, method level, shared, constant variables do make a difference. If the coder in question cares at all about his/her profession they will understand and try to do the right thing.

Answer 17

My solution when dealing with outsourced resources that didn't give a &%$# about formating (or easily preventable bugs for that matter) was to have the build server enforce this. I created a CI server job that ran every night that checked the code out, ran Jalopy and findbugs then checked the code back in. Once the other team learned that not using the standard code conventions would make their job harder, they started to use their IDE to maintain a standard format.