23

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.

7

4 Answers 4

34

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.

16
  • 3
    Agreed, even the most poorly named function can be recognized and remembered if you use it long enough.
    – JeffO
    May 8, 2014 at 18:24
  • 29
    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, 2014 at 20:15
  • 6
    -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. May 8, 2014 at 21:00
  • 3
    @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. May 8, 2014 at 21:04
  • 3
    @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." May 8, 2014 at 21:09
14

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: https://softwareengineering.stackexchange.com/questions/78100/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

3
  • 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, 2014 at 22:10
  • 1
    Your blog post is full of good advice save for the "know your place" comment. This is badly conceived advice in my view. Anyone new to a group can bring good ideas to that group and so the group should never adopt a "you're too young to have an opinion, so be quiet and learn from us" attitude.
    – David Arno
    Nov 23, 2015 at 19:19
  • 1
    Collaborating and making code readable for everyone is not policing - it's about communication. Unfortunately a lot of senior or "smart" coders bypass that little detail when writing code.
    – Pithikos
    Jan 16, 2019 at 20:07
6

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

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

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