What's the right balance between code consistency and code improvement?

Question

Recently I had a discussion with a colleague regarding code style. He was arguing that your usage of APIs and the general patterns you are using should be as similar as possible with the surrounding code, if not with the the codebase as a whole, just as you would with code appearance (brace positioning, capitalisation etc). For example if I were adding a method to a DAO class in C# I would try to use LINQ where appropriate to help make my code clean and easy to maintain, even if none of the other methods in that class were using it. However, my colleague would argue that I should not use it in that instance because it would be against the existing style of that class and thus harder to understand.

At first I found his position rather extreme, but after thinking it over for a while I am beginning to see his point. With the hypothetical LINQ example, perhaps this class doesn't contain it because my colleagues are unfamiliar with LINQ? If so, wouldn't my code be more maintainable for my fellow developers if I didn't use it? On the other hand, if I truly believe that using such a technique would result in cleaner code, then shouldn't I use it even if it differs drastically from the surrounding code?

I think that the crux of my colleague's argument is that if we all go about implementing similar functionality in a codebase in different ways, and we each think that our way is "best", then in the end the code as a whole just gets harder to understand. However at the moment I still think that if we blindly follow the existing code too much then the quality will just slowly rot over time.

So, to what extent are patterns part of code style, and where should we draw the line between staying consistent and making improvements?

Answer 1

To give a more general answer:

In a case like this, you have two programming "best practices" that are opposed to each other: code consistency is important, but so is choosing the best possible method to accomplish your task. There is no one correct answer to this dilemma; it depends on a couple factors:

• How beneficial is the "correct" way?

  • Sometimes the new and improved best practice will dramatically increase performance, eliminate bugs, be far easier to program, etc. In such a case, I would lean heavily toward using the new method. On the other hand, the "correct way" may be little more than syntactic sugar, or an agreed idiomatic method of doing something that is not actually superior. In that case, code consistency is probably more important.

• How big of a problem would inconsistency create?

  • How interconnected is the new code with legacy code? Is your new code part of a library? Does it create an object that gets passed to many parts of the program? In cases like these, consistency is very important. Using a new API, or a new way of doing things in general, might create subtly different results that break assumptions elsewhere in your program. On the other hand, if you are writing a fairly isolated piece of code, inconsistency is less likely to be a problem.
  • How large and how mature is your code base? How many developers need to understand it and work on it? Agreed-upon, consistent standards are much more important for larger projects.
  • Does the code need to run in older environments that may not support the latest features?

Based on the balance of these issues, you have to make the right choice about which route to take. I personally see little value in consistency for consistency's sake, and would prefer to use the latest, best methods unless there is a significant cost to do so.

Of course, there is a third option: rewriting the existing code so that it uses the best methods and is consistent. There are times when this is necessary, but it comes with a high cost.

Answer 2

Staying consistent has little value in my perspective; continuously making improvements is a must.

Your colleague's position really impedes innovation. The consistency argument gets you into a situation where you can use, for example, LINQ only if you migrate all code to use LINQ. And well, we don't have time for this, do we?

I'd rather have inconsistency where some code is still doing foreach over ArrayLists and other parts use LINQ on IEnumerable<T>, instead of sticking to the oldest way of doing things until the end of time.

It's your colleagues' responsibility to stay relevant and learn new ways of doing things.

Answer 3

API consistency is very important, both for public and internal APIs.

Code formatting consistency is important, and should ideally be enforced by automatic formatting tool with same formatting rules for everybody. Makes living with shared version-controlled codebase easier.

Naming conventions should be consistent, also for things like local variables etc.

Static analysis tools should be used to enforce certain other conventions and good practices (but do not blindly follow the defaults of any such tool, the defaults can sometimes border on insane), though if something demands it, don't be afraid to disable some check (usually with a directive inside a comment) temporarily, if you can justify it.

What happens inside functions/methods, apart from what is listed above, doesn't need to be consistent in any way, as long as it is good code on its own. Good, understandable, well commented code is very important, but after that, if compiler and static analysis tools think it's consistent code, that's consistent enough.

---

About new language features such as LINQ (well, that's not exactly new), the only thing that needs to be considered is, will new enough version of language/libraries be in use everywhere where the code will be used? If in doubt, stick to features which are known to be compatible. This doesn't of course prevent you from pushing a version upgrade throughout the system, so you can start using the new nice things.

And every developer working with code should keep up to date, so any .NET developer should know LINQ, and if they don't they should be forced to learn, for their own good (you never know when you will be looking for a new job in this business, for one reason or another).

Answer 4

The answer to this is simple.

Consistency is of paramount importance.

but it comes with a caveat...

You and you coworker are likely obsessing over the wrong sort of consistency

Implementations are disposable. They can be completely overhauled with varying degrees of ease depending on the quality and comprehensiveness of the test suite. Worrying about things like "Should this be a property?", "Shouldn't thins code use LINQ instead of a lower level construct?" is of dubious value. Its vary hard to tie any measurements to the value of consistency at the implementation level. A much better question to ask at this level is "Does this code work as advertised?". TL;DR implementation consistency is where "little minds" get their hobgoblin on.

Why isn't consistency as important here? Implementations usually have a tiny number of contributors. Most methods are written and never touched again. Of the remaining code the number of methods that have two contributors almost certainly a majority. This pattern continues ad infinitum. In this context consistency just isn't that important. If the shelf life of the code is pretty small(a few years) the gains from aggressive consistency are likely a non-factor.

This is not to say that you should go crazy in your implementations. Rather its to say that a nice, clean, simple design will be orders of magnitude more valuable to your hypothetical future maintainer than silly boiler plate consistency method by method. This takes us to the real point...

APIs are not disposable.

This is all APIs code level, web services, SDKs etc. These must, must, MUST be consistent. The productivity gains from this variety of consistency are enormous for a number of reasons:

Integration Tests:

If you keep your API's consistent you can create suites of integration tests. These allow developers to freely swap the implementation details and achieve immediate validation. Wanna swap your co-works crap over to LINQ? Do the integration tests run? It also provides validation when preparing to go to production. Because computers are fast a single laptop can preform the work of a thousand testers executing mundane tasks. Its tantamount to increasing your organization's headcount considerably.

Productivity

When API's are consistent you can make guesses about how to use an API just by following what you've learned about using other parts of the API. This is because the API affords a natural, consistent "look and feel". It means your client's spend less time sifting through documentation. Onboarding is easier and cheaper. Less questions are asked to the people that developed the API. Consistency makes everyone a winner

Why is consistency important in this scenario? Because API's have the exact opposite problem of implementations. The number of people using them is typically much greater than the number of people contributing to their implementations. Small gains from a little consistency are multiplied and costs of maintaining that consistency are amortized.

Conclusion

Consistency is expensive. On its face it lowers productivity. It constrains developers and makes their lives harder. It places limitations on the ways they can solve a problem sometimes forcing them to solve it in a non-optimal way. This is often for reasons they don't understand, are ill-conceived, or they are not privy to(contracts, larger organizational or inter-organizational policies).

Raymond Hettinger made some excellent points in his Pycon 2015 talk regarding using the PEP8 style guide for teams of python programmers. He showed that the obsession on stylistic consistency on a piece of code caused code reviewers to miss serious logic and design flaws. His hypothesis can be summarized as finding stylistic inconsistencies is easy; determining the real quality of a piece of code is hard

The point here is be critical. Identify where consistency is important and protect it aggressively. Where its not important don't waste your time. If you can't provide a objective way to measure the value of consistency(in the above cases "effective headcount", cost as a function of productivity) and you can't demonstrate the returns are substantial then your are likely doing a disservice to your organization.

Answer 5

Unfamiliar with LINQ? If so, wouldn't my code be more maintainable for my fellow developers if I didn't use it?

The C# language is still evolving. If people didn't learn the changes from C# 1, they would be missing out on:

• Generics
• Partials
• Anonymous methods
• Iterators
• Nullable types
• Auto-properties
• Anonymous types
• Extension methods
• Ling
• Lambdas
• Asynchronous methods

This is just a small selection of common features found at the Wikipedia article. The point I'm making is that if the developers don't learn, the codebase will stay in a vacuum. One would hope that your developers do continually improve and the code base evolves, supported by a complete test suite. Do you remember how bad it was with .NET 1 to manually implement properties.

Linq makes life easier. Use it where you can. You might motivate your team members.

So, to what extent are patterns part of code style, and where should we draw the line between staying consistent and making improvements?

Improvements are gradual. To me, it makes no sense to keep old style code which is less readable and potentially less maintainable. Your team members should at least be able to work out what's going on, even if they can't write it.

Answer 6

You should be consistent, but not necessarily with the old code. That is, your team should agree on the right way of doing something, and use that way whenever new code is written or substantial changes are made.

That way, you reap most of the benefits of consistency (in particular, it won't matter who writes the code), but can still improve if the team sees a clear benefit by doing things another way.

In an ideal world, we would rewrite all old code to conform to the new right way. While this is not economically viable, it should make technical sense (or else, the new way is not a better way), and your long term plan should be to upgrade it. If that is not worth it, don't bother with the new way. Put differently, there must be a clear advantage to the new way to consider declaring it the right one.

Answer 7

I think it is important to follow a code style in a project eg. naming conventions, indentations etc.

I do not agree that you should be limited from using new language constructs or design patterns simply because your colleagues do not understand them, or they haven't been used in the project before.

Of course these things should have good reasons for using them eg. performance or brevity, if appropriate.

Answer 8

I would be extremely careful blindly following current design patterns. Of course, when there is no appreciable downside, one should always attempt to follow similar patterns across the code base.

However, it's far too easy to get into circumstances where the majority of developers feel as though it's "best practice" to following the existing bad patterns leading to situations where good developers are writing bad, unmaintainable code.

On the other hand, consistency is important. When dealing with enormous code bases, it's extremely useful to try to follow similar patterns so that even though developers have not read every inch of the code base, they know what to expect.

Therefore, I believe it's best to establish and update guidelines about what patterns developers should follow if able. This way, any new code is consistent with other new code.

Answer 9

We have regular, fairly informal, technical meetings which any one of us can run. If we find a new technique we present it at the meeting. You generally get a good feeling of how well the idea is accepted and understood by your colleagues. This helps you to decide whether it is wise to include it in your code, helps others to decipher it if you do and also establishes the 'go to' person if others need help with that style. If no one reinvented the wheel we'd still be dragging stuff around on logs.