Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

A co-worker in the small start-up I work at writes (C++) code like this:

// some class
class SomeClass {

    // c'tor
    SomeClass();

    // d'tor
    ~SomeClass();

    // some function
    void someFunction(int x, int y);
};


// some function
void SomeClass::someFunction(int x, int y) {

    // init worker
    m_worker.init();

    // log
    LOG_DEBUG("Worker initialized");

    // find current cache
    auto it = m_currentCache.find();

    // flush
    if (it->flush() == false) {

        // return
        return false
    }

    // return
    return true
}

This is how he writes 100% of his code: a spacer line, a useless comment which says nothing other than what is plainly stated in the following statement, and the statement itself.

This is absolutely driving me insane.
A simple class written by him spans 3 times as much as it's supposed to, It looks well commented but the comments contain no new information. In fact the code is completely undocumented in any normal definition of "documentation". All of the comments are just a repetition of what is written in C++ in the following line.

I've confronted him several times about it and each time he seems to understand what I am saying but then goes on to not change his coding and not fix old code which is written like this. I've went on and on again and again about the distinct disadvantages of writing code like this but nothing get through to him.

Other co-workers doesn't seem to mind it as much and management doesn't seem to really care.

What do I do?
(sorry for the rant)

share|improve this question
1  
Have you tried the "write it like the person maintaining it is a maniac with psychotic tendencies who knows your home address" approach? –  Blrfl Nov 5 '12 at 21:17
1  
@JessicaFriedman: we feel for you then :) Put him out of his misery and slap him silly with his keyboard for me the next time he comments a constructor with just c'tor. There's seriously no excuse for that one. –  haylem Nov 5 '12 at 21:53
1  
@FrustratedWithFormsDesigner you're speaking out of my mouth! –  Jessica Friedman Nov 5 '12 at 21:54
1  
How the hell does he mean that it makes it easier for him to read the code? The comments are identical to the code, does he struggle reading the code? Can he code? I'm confused, frustrated and a bit angry about this! Edit: What he actually does here is undermine all the important comments. I, and most likely your teams developer, would be so accustomed to see crappy comments that you just ignore them. When a real important comes along, you will ignore that too. A comment should be a red flag, not spam you just ignore. –  martiert Nov 5 '12 at 22:03
1  
@martiert: I'm quite shocked as well that that guy would even say "just ignore it". The nerve :) I'd write a commit-hook script to erase all single 1-line comments in this code. Might even be worth losing the odd good and useful ones! Or if I don't have the rights on the SCM server, just a script that regularly updates locally and does changes before re-committing a change. –  haylem Nov 5 '12 at 22:07
show 13 more comments

closed as not constructive by gnat, Robert Harvey, ChrisF Nov 5 '12 at 21:52

As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, visit the help center for guidance.If this question can be reworded to fit the rules in the help center, please edit the question.

4 Answers

There's really not a lot you can do unless

your

organisation

has

coding

style

guidelines

share|improve this answer
3  
There are coding style guidelines but none of them says "don't write code like a douchebag". When I tried talking about such guidelines, he goes on saying "But nobody here abides the guidelines". How can you even argue with someone like that? –  Jessica Friedman Nov 5 '12 at 21:08
    
@JessicaFriedman: Is there a formal code-reivew process? That would seem like the logical place to start enforcing the guidelines. –  FrustratedWithFormsDesigner Nov 5 '12 at 21:14
2  
@JessicaFriedman: easy, by persuading a manager that having some static analysis tools and metrics generation on each build would help to monitor quality, which will have an impact on the number of bugs, which will have an impact on testing and support costs. And in the middle of the list of violations for said analyzers, you put in what you want to eradicate. Or even better (but extremely annoying to the point of being counter-productive), you persuade them that pre-commit hooks are a good thing and you reject non-complying code. I'd say the latter is a not so good idea, but the former works. –  haylem Nov 5 '12 at 21:23
    
@FrustratedWithFormsDesigner no formal code review process. I'm interested to know what tools people use for this... –  Jessica Friedman Nov 5 '12 at 21:50
1  
@JessicaFriedman: lots of them. For reviews, you can use Crucible or ReviewBoard, or Rietveld. For CI systems, it varies on the language. Jenkins+Sonar's pretty nifty combination for the Java world, but can be bent to work with C or C++ as well, though I'm sure there are other alternatives. –  haylem Nov 5 '12 at 21:52
show 2 more comments

Start a pre-checkin peer review process. Use a code review tool. Start community discussions around style. Management doesn't care about code style, but they LOVE peer reviews and metrics.

Of course, if everyone prefers the obnoxious style, you could be forced to start coding that way. :)

share|improve this answer
1  
My previous work place had a very strict code review protocol and that kind of stuff would not have passed it in any way. Unfortunately in this start-up, code review is considered by most a waste of time. –  Jessica Friedman Nov 5 '12 at 21:58
add comment

Dealing with Douchebags: Your Boss's Job

Not yours. Learn to either ignore him, or report him with evidence that his attitude not productive and that his coding guideline is detrimental to the codebase's quality.

Dealing with Existing Bad Code: Your Continuous Delivery Pipeline's Job

  1. Set up Continuous Integration system to do periodic builds.
  2. Set up a continuous integration game to attribute good and bad points on broken builds.
  3. Set up continuous inspection system to calculate metrics on the codebase and flag obvious violations.
  4. Set thresholds in both systems so that some builds will be marked as FAILED if some thresholds are way too off the charts.
  5. Show pretty graphs to managers.
  6. Gradually hunt down crap code and axe it.

Refusing New Bad Code: Your Team's Job

  • Set up regular peer-reviews for large features;
  • Open tickets for required changes;
  • Implement pre-commit hooks that you can buy to similar analyzers as for the CI systems, and rejects commits that are also off the charts fugly.

Note: This is all a bit petty and I'm definitely NOT saying that I believe defect counts, code metrics and so forth are necessarily a good indicator of a coder's and a codebase's virtue, but it helps to get the point across and bring, enforce and maintain good habits.

share|improve this answer
    
Good stuff here. What should I use for code metrics? –  Jessica Friedman Nov 5 '12 at 21:52
    
@JessicaFriedman: obviously I'd leave out the one about "percentage of commented-code", as it might not be a strong-point for your case... :) A general code quality index is generally what matters the most to a manager, based on a list of other metrics. Usually, the driving argument would be that you reduce the likelihood of bugs by reducing the "surface" for them. And having noise in your files definitely makes it worse to keep an eye on that surface. So having metrics that actually show a decreasing number of LOCode on an existing code base shows you're doing an effort to cut away crap. –  haylem Nov 5 '12 at 21:57
    
@JessicaFriedman: It's going to take time, though. That sort of stuff won't help you make a point right away. It will need to be done over time, and you'll need to gather information regularly (hence the idea to link it to builds) and show that as you worked to reduce said crap (by using your code metrics), the software's quality also improved (by using your defect counts, customer report counts, etc...). A bit more petty as well, but if you can manage that more bugs hide in that person's code than others, obviously that would help. But don't make it a witch-hunt. –  haylem Nov 5 '12 at 21:58
    
@JessicaFriedman: I also wonder if some of these tools would count all these identical comments (like // c'tor) as duplicated code... If yes, then that'd be a good metric for your case as well. –  haylem Nov 5 '12 at 22:05
add comment

Not that this helps you, but I've seen this done as psudo-code prior to writing the actual code. To be honest I've used it for more complex stuff in the past, but some people are fairly rigid in there habits and will follow the same steps for all their work.

Without a standards document you really are out of luck. You might want to consider changing your organization.

    // some class
    class SomeClass {

        // c'tor
        // d'tor
        // some function
    };


    // some function
    void SomeClass::someFunction(int x, int y) {

        // init worker
        // log
        // find current cache
        // flush
        // return
    }
share|improve this answer
add comment

Not the answer you're looking for? Browse other questions tagged or ask your own question.