How clean should new code be? [duplicate]

Question

I'm the lead designer in our team, which means I'm responsible for the quality of the code; functionality, maintainability and readability.

How clean should I require my team members' code to be if we are not short on time?

In my view, we should clean up old code we modify; adding a line to a method means you clean up that method.

But what about new code?

We could make it sparkling clean, so that if another coder comes along tomorrow and makes a small modification, she doesn't have to clean it up at all. But that means if no-one ever reads that piece of code again, we've wasted time making it sparkling clean.

Should we aim for "almost clean" and then clean it up further on future visits? But that would mean not getting the full value for the understanding we had when we wrote it in the first place.

Right now, I'm going for "sparkling clean"; partly as a tutorial for my colleagues who are not as picky as I.

Answer 1

My experience:

1. A piece of code longer than 20 lines, written as a proof-of-concept, will not get re-written when the concept thus proved is needed, but rather bent to somewhat fit the needs of the production code.

2. Usually, this goes with the promise to re-write it later, when there is time to do it all properly. However, this time never comes, so this piece of test code will be stuck in production code forever.

3. Any software project larger than, say, 2 man months stands on the shoulders of at least one piece of such proof-of-concept toying-with-the-idea code, big projects are built on many of them. And nobody is going to replace these pieces of code which usually are underlying vital features of the application, threatening to bring it all down on a single error.

4. Too many big projects sooner or later trip over such code.

My solution:

Anything over 20 lines of code has to be rock-solid and bullet-proof. Properly design the whole thing, do not use hacks to save time, put effort into picking good identifiers, refactor as soon as you see the need,... The whole enchilada.

While 80% of these snippets are indeed thrown way, the rest will end up at the heart of some code that keeps the company afloat a decade down the road. That prospect excuses any amount of effort poured into a small, seemingly one-off, program.

Answer 2

In my experience, making new code clean is actually not that difficult. Typically you have a single clean concept in your mind about what the code is supposed to do, how to implement it etc. so it is fairly easy to arrange the code to reflect this one thing clearly (a least according to one's current understanding - returning to the same piece of code 6 months later may reveal some surprises ;-).

The problem comes when the existing code needs to be modified / extended. Then it starts to get more complex, the original clear architectural / design vision is getting blurred, so we need to spend conscious effort to refactor and keep code entropy under control.

Creating a piece of code as clean as it could be would certainly make future extensions and maintenance easier. If there are future modifications, that is. Otherwise, as you note, the effort is "wasted" - at least in the direct sense. However, there are additional things to consider:

• That a piece of code has never been modified up to now does not mean that it never will be
• Any piece of code is typically read many more times than it is modified
• It is better to get used to writing clean code as a habit, rather than trying to decide every time whether this particular piece of code is important or critical enough to be made cleaner. Thinking about this may take more time than actually improving the code, and some developers may easily start using this rule to justify their laziness or resistance to the new ways
• The more you practice a skill, the better you get at it*; and writing clean code is important enough to be practiced constantly.

All in all, thes arguments sort of lead to the opposite of YAGNI: in this case You Are Gonna Need It. If you have the time, write as clean code as you can. (But no cleaner - at some point the rule of diminishing returns kicks in, so we should stop and find some better thing to do. To avoid endlessly polishing the same 3 lines of code we need to focus on meaningful changes which represent real improvement in readability / maintainability.)

*_{I haven't heard any healthy adult complaining about how hard walking is, but witnessing a baby or an injured person trying to (re)learn it makes you realize how complex an achievement it actually is. Still, most of us does it all the time without even consciously thinking about it.}

Answer 3

It depends. I would make two cases, one for new code and one for existing code.

For new code I would recommend using test-driven development (TDD) plus merciless refactoring as standard practices. Make it as simple as you possibly can by refactoring and stop only when you cannot think of any improvement anymore. While this may sound like a lot of effort at the time you will find that it pays off times and times again. In other words, yes, make it "sparkling clean".

For existing code, however, I would be much more careful. It depends on the quality and coverage of your test suite. Typically existing code is code that has been in use for quite some time. This also means probably most of the bugs have been fixed. When you now start changing this code you want to make sure you don't break anything that works. My recommendation would be to write tests first. Heaps of them to ensure that you clearly understand what the existing code does but also to ensure when you start cleaning it up you don't accidentally break anything.

Your choice of approach also depends on what tools you have available. Refactoring and unit test support can be quite different for different languages. Make sure you get the best possible tools. Regardless of price, they are worth it.

There are more aspects to working with legacy code. I would recommend to consider reading Mike Feather's book "Working Effectively With Legacy Code". Good luck!

Answer 4

Over the past few years, I've discovered an iron rule:

Code always lives longer than you expect.

That one-shot throwaway thing you threw together in half an hour so that marketing could modify some text on a non-critical page designed for two weeks of production use will probably still be in use when you leave the company two years later. That little works-for-now deployment script you made will be used until the entire project dies. That little we'll-use-this-until-the-real-thing-is-finished replacement for the commercial and now defunct third-party ORM component you made may very well become the real thing itself.

Also, clean code does not take longer to write. It does if you write a truckload of quick-and-dirty code first and then clean it up, but if you start out clean and keep it clean along the way, it is often even faster.

A really good musician once told me: "Always play the best you can. Even when you're doing some warm-ups in your hotel room, give it all you have. You never know who's in the room next door listening."

Answer 5

For each programmer on the team, the answer should always be "a little cleaner than the code I wrote last week."

Too often, having a specific hard rule in an organization's coding standards leads to a disconnect between what is officially supposed to happen and what actually happens. Most programmers of average skill can't or won't suddenly change their ways overnight, and even if they do hit the standards regularly, they will have their off days. In the real world, it's more practical to focus on improvement than perfection.

Answer 6

Always go for as clean possible at the time of writing. Saving time writing sloppy code is a false economy, a price is paid for every short cut every single time you fix a bug or modify your code.

Refractoring old code is less clear cut and depends on the judgment of the coder at the time. Sometimes its "right" to write in a less clean style that's consistent with other code. Refractoring existing code is both hard and time consuming. Sometimes its not festable to rewrite large chunks of working code for one modification.

Answer 7

The key is to clean up the code you are most likely to work in again.

Working in well-factored code is more fun and more likely to produce the desired result. Those are both important.

However, refactoring takes time, and all edits carry risk of new bugs. Refactoring code that you don't otherwise need to work in is a bad idea.

How to know what code you need to work in? Changes tend to happen together, in high-churn areas, while other areas are left idle for a long time. There are many reasons for this phenomon; one is that the changes you make right now are probably wrong, but you can't know that until they're done.

So, I use the following sequence when working on a feature or bug fix:

1. Imagine what the code would look like if it was ideal for the change I want to make, so that the change would be a simple one-line edit that was obviously correct?

2. Imagine a path of refactorings to get things to that point.

3. Make one refactoring at a time, testing as I go, and checking in each refactoring separately, with a special change description that starts with "REFACTORING:". This helps make it obvious that if you're looking for a deliberate behavior change, you don't have to look at this one. Also, since most refactorings are well-defined with well-known names, it is easy to understand these changes later.

4. Make the single one-line edit to get the desired functionality change.

This approach works just as well for changes to existing functionality as it does for adding new functionality, as everything is existing code after day 1. However, if you're branching in to a new area of functionality for your program, it may make sense to throw a quick-and-dirty implementation together to get real feedback on your plans.

There are many caveats:

• If a deadline is looming, the risks of refactorings may outweigh the benefits. Judgement is required.

• Sometimes the code would require extensive refactoring to make this work, but it would be possible to implement the desired feature or bug fix easily enough. In this case, I may do the more obvious, straightforward refactorings but not the ideal described above.

• If I don't have high-quality automated tests, the risks of refactoring are more concerning. I may refactor less, depending on the circumstances. However, poorly-factored code is also difficult to unit test, so it's easy to end up in a quagmire if you're not careful.

Note that I avoid refactoring towards structure that I think will be beneficial for some future change that I can't be sure I'll need. Refactor for the issue at hand only. However, if this is not the first time I've needed to make this kind of change, I may broaden the scope of refactoring in anticipation of future similar changes. Judgement required, again.

Answer 8

From Refactoring: Improving the Design of Existing Code, p58;

1. The first time you do something, you just do it.
2. The second time you do something similar, you wince at the duplication, but you do the duplicate thing anyway.
3. The third time you do something similar, you refactor.

Answer 9

I've always had a drive to not repeat myself in code--I consider it the single most important drive in good code--nearly everything else derives from this.

In doing so I've found that refactoring code to eliminate redundancy is one of the most educational practices as well. Over time I've learned to recognize more things as redundant as well as more patterns and tricks to use to refactor them away.

On top of that, code-combining refactorings stack like you wouldn't believe. You start at the inside and refactor a handfull of similar statements to use a data structure, that allows you to identify similarities with other pieces of code and it rolls uphill with surprising speed, reducing code by the bucketful on the way.

So why wouldn't you do this wherever you had the chance, even if you didn't have lots of extra time?

Most of the extra "Clean" in your code such as formatting and comments comes from this process as well--who wants to document 5 pages of repeated code? As you isolate them into a single function, NOW they are worth documenting!