1

I'm currently working on a large project in C++. The style guide for this language has been well defined by my company and is available for everyone to see. This particular code-base is being developed by two separate teams and has been developed by many different people over time.

Much of the code written by developers does not follow the company style guide, each developer has applied some of their own style in naming, comments, formatting etc. When writing new code should I follow the company style guide (but have code that looks stylistically out of place in many of the source files) or do I try to mirror the style of the code I'm currently working with?

  • 1
    @gnat Would a suitable answer to my question therefore be: "Have someone peer review the code and make a decision?" – innova May 28 '15 at 11:05
  • yes. No matter which way you pick, if your peers keep complaining about your way of doing things, be it one way or another, you better change to make them feel better – gnat May 28 '15 at 13:01
6

First, you say that there are two teams currently working in the code, so the obvious answer is "ask whoever is in charge". It makes no sense doing the effort of adapting the code to the company styles if the other teams do not try to adapt it, too.

If you were alone maintaining some other people old code, probably most of your time you will be modifying only a small portion of it (there are parts of an IS that change more than others). There, I would go this way:

  1. First and foremost, make it visible. Talk to your boss and ask him what to do; explain him that you sometimes will last a little longer doing changes but in the long run it will be profitable. He will decide if he cares most for short or long term.

  2. Do not get sucked into a full refactoring! A good rule of thumb is restricting the changes to methods/classes that you have modified as part of the system changes. If you need to do "secondary" changes due to refactoring (v.g., modifying other class because you changed a method name due to refactoring), make those secondary changes but do not touch anything more.

  3. Clean the parts of the application that you modify more first. No need to modify code that "just works" and nobody is going to read again.

  4. After doing the functional changes and before doing the cleanup, do a commit in case something goes wrong (unmatched brackets, etc.).

  5. (If rebuild time is low enough) If you make changes that affect other parts of the code (name of classes or methods), make the changes, recompile the system and decide if the change is worth changing all those "symbol not found" error.

  6. Beware of inheritance! Remember that if you modify a method in a class, maybe there are no symbol not found errors because the compiler is using the parent class implementation of that method!

  • +1 for restricting changes to code you are already modifying. If you restyle code that someone else is working on, you'll get annoying merge conflicts when your style changes conflicts with their actual changes. – Idan Arye May 28 '15 at 12:37
1

I think it depends on a lot of social factors. For one, does it even affect you or your position if you do not adhere to the style guide? After all, a lot of code has been written by others with this guide in place already. If it does not really affect you, do you (for whatever reason) want to champion the style guide?

Most of the times, I found that once a style guide was ignored by one or more teams without serious repercussions, then the guide may as well be removed. It simply is not sufficient to write a document that explains the style guide, one needs to enforce it as well.

Hence, a more fundamental issue is present here: Does your company's style guide still have supporters, and why did they not enforce it?

You can think of it the other way round: You wouldn't have even bothered with this question, if your boss told you that must write any and all code for all projects in correspondence with this style guide. Apparently though, you have doubts as to the applicability of the guide instead.

In summary, what can you do? Given the above, these are a few possible ways, but in the end it is up to you to choose:

  1. Ignore the style guide completely, because everyone else does so as well, and matter-of-factly it is no longer relevant.

  2. Silently apply the style guide to your modifications and if someone complains simply point to the guide. You're not doing anything wrong, but you're also not doing much more than the minimum.

  3. Champion the style guide. Bring up the problem that this code has ignored the guide, and that this fact in itself, is a problem that should be addressed. Get your voice heard and be part of the group that actually enforces the style guide, because you think it is important.

  4. Challenge the style guide. Almost as above, but you don't agree with the guide and want to get rid of it.

  • It's likely the style guide was adopted after most of the existing code was written. – Karl Bielefeldt May 28 '15 at 12:44
  • Maybe, but if you adopt a style guide after you already have a differing code base you would assume there is a rule for how to deal with exactly the OP's situation. – Frank May 28 '15 at 12:47
0

Conforming to the style guide is important, but unless you have the necessary resources to adapt the entire code base to the standard on paper, it could create more disorder than it removes. In general, I wouldn't reformat less than an entire file merely to conform to any standard, and then only if it isn't clearly a business-value-negative proposition (i.e. it costs more than it will ever earn in productivity).

0

If all the code is in one consistent style, or if the code base is very big, or if there's no budget, then I choose the battles I can win and adapt to the other style.

If I'm not the only person actively working on the project I would talk to the other developers and reach a shared agreement.

I would aim at writing all the new code following the guides. Any line I touch should follow the style guide, but I would not mix commits where I change the logic in one line and just the style in another part of the code. It makes the review process very boring and confusing.

If I have some easy wins (like find / replace), I would coordinate with the team actively working on the code and apply them in bulk, then send a pull request containing only those changes. This should get merged asap, or I might end up with tons of conflicts.

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