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.

This question already has an answer here:

Our team consits of junior and senior developers. Problem I am facing is with the code written by seniors. They are not following MINIMUM coding standards. I am also still learning but I wouldn't declare the inline style or give improper names for variables and controls. I tried to mention it indirectly but they were like we can take it up later and since its agile, we should move on. But my problem is when I start working on my task, it makes me feel bad and I would focus more on refactor rather than on my task. I am not sure how to deal with this situation.

PS: I am sure this question has been asked many times. But any advise is much appreciated.

share|improve this question

marked as duplicate by Bart van Ingen Schenau, GrandmasterB, MainMa, GlenH7, MichaelT May 9 at 0:03

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

4  
2  
Agile needs to be used as an excuse/reason to make the code better and not worse. –  JeffO May 8 at 18:23
3  
thecodelesscode.com/case/94 –  Kevin May 8 at 20:17
    
An unenforced standard is merely a guideline. Treat it as such, while lobbying for tools to fix it and enforce it. –  bishop May 8 at 23:26
1  

4 Answers 4

up vote 18 down vote accepted

A couple of things.

  1. Don't assume that your seniors don't know what they're doing. They may have very good reasons why they made the decisions that they did; ask them why (in a non-argumentative way).

  2. Code that is already written, backed by unit tests, and declared functionally complete by your superiors can be safely ignored. That's what you should do with it: ignore it, until it becomes necessary to further maintain it, for whatever reason.

  3. Based on your cursory description, it sounds like your team is under significant time pressure. Compromises are made under those conditions; that's just the way it is.

Choose your battles wisely. Only fight the ones that need fighting.

share|improve this answer
2  
Agreed, even the most poorly named function can be recognized and remembered if you use it long enough. –  JeffO May 8 at 18:24
6  
Even under time pressure the seniors should still follow coding standards. Unfortunately the trump card of "not enough time" gets played too often in our industry. –  Brad Cupit May 8 at 20:15
1  
-1. because of (2) Code that is already written that has tests but is hard to read / hard to use / way too big / tests are really crappy / a big friggin mess ... should certainly be refactored. Constant code improvement is one of the ways to avoid just keeping racking up debt. Code with tests is not automatically "ok". We constantly do this - mixing in re-writes and improvements at the same time as new features and bug fixes. This is a topic and conversation however that you need to continue to have almost weekly with the product owner. –  Michael Durrant May 8 at 21:00
1  
@MichaelDurrant: I don't know of any software project where people regularly go in and massage code that already works, unless they're adding a feature, fixing a bug, or deliberately setting aside time to work on technical debt. If your premise is that we throw the thing together and fix all the problems later, I submit to you that is not the best possible approach. If it's worth doing, it's worth attempting to get it right the first time. –  Robert Harvey May 8 at 21:04
2  
@JohnBarça: Nobody is arguing that. I certainly didn't make that assertion in my answer. But there has to be a point where you say "it's good enough," without engaging in endless, ongoing tweaking. That's the place that you're at when your superiors say "don't touch this; it already works, and there's no point in putting further effort into it." –  Robert Harvey May 8 at 21:09

They are not following MINIMUM coding standards.

If it's not your job to police the coding of others, then don't do it. It's a classic way for juniors to make a bad name for themselves.

See this question for a similar discussion: How do I tell a senior programmer that I disagree with him

Related: A blog post I wrote about disagreeing, and part of that is knowing your place and when it's appropriate to disagree and when it's not: How to disagree without being disagreeable

share|improve this answer
3  
Me, I’m still working on the reverse situation: how to tell one’s juniors that their code isn’t good enough but doing so in a way that doesn’t harm the relationship and which has a chance of getting code out of them in the future. It doesn’t matter that management has specifically requested that I do this (they have, in fact); it’s still delicate, and the results never assured. –  tchrist May 8 at 22:10

As others have mentioned, you should ask about the senior developer's choices in a non-confrontational way, but it might also be worth it to look into a few things.

First of all: is there a coding standard that the project follows? If there is no coding standard defined, then nobody is breaking the coding standard. The 'standard' may simply be to make new code look similar to the old code. That's an unfortunate position to be in, but an ad-hoc coding standard is still a coding standard.

If there is a coding standard: who ensures its implementation? Are all developers responsible, is it handled during review periods, etc.

I'll give you my example: I came onto a project that's just starting out. We have senior members, but mostly juniors(myself included). As it turned out, I was the one who was most concerned about a coding standard. Our company and QA policies dictate that we have a standard, and since I was the one who brought it up it was my job to find one.

That turned out to be super easy. There's a standard for Java set by Oracle, which makes that the obvious choice. Next, I found a tool for Java called Checkstyle, which enforces coding standards for you. Every time you make a change and save a file, Checkstyle runs over the file for any violations of the coding standard you have, and high lights all violations just like a warning. Additionally, it does a few static analysis tasks: N-Path complexity, cyclomatic complexity, etc. After that, I looked into Eclipse's formatter and found I could make that conform to our coding standard as well. So now, after a minimal amount of set up, we have a tool that:

  • High lights coding standard violations
  • A formatter that can be invoked with ctrl + shift + f
  • Zero maintenance
  • No time is spent in code reviews for trivial coding standard issues(tab here, space there, comment needed here, etc)

So, ultimately, to really deal with maintaining a coding standard, I would:

  1. Ask what the current standard is, is it defined somewhere, and are all developers aware of it?
  2. Get a plan ready - do some homework and search for tools in your project's language that can help you do what I did. Seriously: having a coding standard in place that everyone follows is great. I ended up making it so easy to do that no one had a reason not to follow it.
  3. Once you have a plan, try talking about it to your supervisor. Tell them your plan, and more importantly: tell them how this could save them time in the future by having fewer defects in the code.
share|improve this answer

Exercise the practice of openness in a professional setting - if you feel that their code may be improved and you have a good reason to assert they should be doing it differently, there should be nothing in your way to suggest it (as long as you aren't condescending or such). If you work somewhere where openness is frowned upon, I don't know if that's particularly healthy. They may have some excellent insight as to why they're doing something that you didn't notice or the opposite - someone is bound to learn something by just asking questions :)

share|improve this answer

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