views:

649

answers:

17

In our company, developers always have dozens of important tasks assigned and tight deadlines in which to complete them.

In this environment, a code review often shows that their program will execute correctly but is "smelly", i.e. hard to read, hard to maintain and a potential breeding ground for bugs. How do you persuade developers (or their managers) that they need to spend extra time modifying their work?

In my experience the long-term benefits of clean code can rarely be justified against the short-term demands of the business and those other high-priority tasks. Does it even make overall business sense to strive for such cleanliness after development time has been spent producing the correct but smelly version?

A: 

Give them a week (or month!) of bugfixing.

By the end of it they'll be so sick of poorly written code they'll make sure they write clearer code in the future!

Good code is always worthwhile (unless it's a throwaway script that will be deleted after it's run).

Greg
+6  A: 

In my experience, managers have to fight to prevent us from re-writing smelly code. They're more focused on getting the next check mark on the feature list than making the code robust and maintainable.

Paul Tomblin
I think it depends on the developer. Some like to spend ages continually refactoring, while others like to get something working as quickly as possible and move on to something else. Both can cause problems.
Paul Stephenson
+9  A: 

It's probably more important to persuade managers rather than developers. Therefore you need some kind of business case calculating the benefit.

I'd suggest to let developers track the amount time they currently spend on fixing code that is "smelly". Thus, a manager can see what the future costs are to fix such hard to mainain code.

poezn
+4  A: 

Ideally, refactoring into pretty code is an ongoing part of the development process, not something to do afterwards.

It is probably hard to convince management to spend ressources on "cleaning up" if you sell it as project seperate from "making it work". Especially if you use technical slang like "cleaning up", "pretty code" and so on, which to non-technical people sounds like you want to paint the inside of the engine in pretty colors.

JacquesB
+3  A: 

It is hard to justify cleaning up the code after the fact if it isn't causing problems (known bugs or lengthening development of new features). Seems like you have two choices - either factor in the code reviews and cleanup before full QA and release or refactor the code later when you need to reuse it or fix major bugs.

Jim
+2  A: 

Code reviews are great for finding the smell, but in the business world it's hard to justify fixing it at this point if the "smelly" code works as the end user doesn't care.

What need to be done is to better educate the people who write the code in the first place. Simple changes like small functions and decent variable names are easy advocate to collegues. However, it can be a problem when the person with the smell issue is senior and isn't prepared to change!!!

Sean
A: 

It woul bevery difficult to persuade anyone to go back and re-factor existing and working code without a good reason to do so.

However it is quite justfiable to make the code quality and cleanliness mandatory for all future tasks. Define formal and verifiable criteria. After that each time you have a change request or bug-fix add the refactoring time for the affected code (and affected code only) to the estimation.

If you just give your developers a week to clean up they would probably spend it less productively then if they will be working on refactoring specific aspects of specific functionality.

Ilya Kochetov
Thanks for the answer. I will vote it up if you fix the typos!
Paul Stephenson
+1  A: 

i guess it depends on how long you want/need the code to last.

if it's to be thrown away in a week no one cares what it smells like. if you need to build on it a week, month, even a year from now you have to keep it clean or the cost of change will be larger than a rewrite.

technical debt incurs interest!

matpalm
+1  A: 

From my experience I developed the practice to fix "smelly" code just after the release.

Let's say you have your brand new version released and you know there is "smelly" code pieces inside. Then you create a branch in the repository for maintenance the just released version and in the trunk the developers can fix the smelly code.

In case the project manager approves such refactoring, you can put this in the planning as separate refactoring task to be done in parallel with the new functionalities for the next release. But in case you can not get the refactoring approved, just make "clever" planning and extend the time for the other tasks, so the developers can fix "meanwhile" the smelly code in backgroung.

But force them to fix it ASAP, just after the release. Don't wait until the next release or milestone date approaches. If the release date is close, then forget refactoring and stick to the working version for that release.

m_pGladiator
+1  A: 

There's the old saw: "If it works, don't fix it." The fear being that the 'fix' will introduce bugs.

The real problem in cases like yours would be either lack of proper standards, or lack of enforcing adherence to those standards.

The only way to make the future code less smelly would be to enforce coding standards.

slashmais
A: 

Do code reviews

Pbearne
We already do code reviews. The question is whether to spend time on the results of those reviews when it won't change the external functionality.
Paul Stephenson
+2  A: 

I am trying to address this very issue at my workplace at the moment. The approach I am taking is...

  1. Persuade people that if "technical debt" has accumulated during a phase of development, it will cost them in future phases, both in terms of bugs introduced and missing deadlines.

  2. Find ways to measure that technical debt. Using tools like FxCop, StyleCop, SourceMonitor, Clone Detective, NDepend, NCover (for unit test coverage) will at least allow some code smells to be quantified. You can then use these tools as part of code reviews, or release reviews to see if the overall 'smelliness' of the codebase is still growing.

Mark Heath
+20  A: 

As others have already stated, the problem is not convincing the programmers... it's convincing management. (If your programmers are writing "smelly" code and enjoying it, then you need new programmers.)

Management is justifiably reluctant: they're in the business of running a business, not of creating beautiful code. If it won't produce a benefit for the company then the programmers can just go home and write beautiful open-source on the free time -- code as a form of "public art" has never really caught on. But there is a business benefit -- the trick is to explain it.

The business benefit of "non-smelly" code is in maintenance costs. Shorter, more elegant code requires less effort to maintain. So the way to convince management is to track your developers' time in two categories: time spent writing original code and time spent maintaining existing code. If you can demonstrate that 5 hrs (or whatever is actual at your company) are spent in debugging and maintenance for every 1 hour of new development, then there is the beginning of an argument in favor of investing additional time and effort in cleaning up the existing code.

Another useful metaphor for convincing management is the concept of "technical debt". This is essentially the same argument (poorly designed code incurs increased maintenance costs) but it expresses it in financial terms -- and if you are trying to convince someone it is often useful to speak their language.

The key thing to remember is that management has no inherent interest in beautiful code (although your developers do -- or should), so to convince them you must address the real issue: the bottom line.

mcherm
+1  A: 

You need to have a list of smelly code, sorted by importance and benefits with detailed explication.

The other thing is to manage code quality from the start of the project. Code reviews is useful to check the state of the code at a point but if it is done too late, the amount of smelly code will be too big and developers will be discourage.

My company developed a quality analysis platform for C# and Java, and we noticed that developers aren't against changing their smelly code if we tell them where and the precise benefit to it.

madgnome
+2  A: 

Seems to me your root problem is the time available to do the tasks in the first place. If they had more time, they would probably write cleaner code. If they have dozens of important tasks assigned and tight deadlines, they will be rushing it out as fast as they can.

Nobody wants to be the guy that is continually late with his deliverables. When it comes to appraisal time the fact his code is clean will not come up. The fact all his code was late will. Similarly if everything is done and working on time, regardless of smell, that's a way to a better pay review.

Bottom line - try and get more time to do each task. Not always easy, I know.

Valerion
You're pretty spot on here Valerion. Getting more time to do each task should be possible because developers estimate the tasks. Occasionally though it can be tricky if the manager asks "Do you really need 5 days for this task? Last time there was a similar one that only took 3 days."
Paul Stephenson
Also it's hard to persuade developers not to be optimistic when estimating tasks. Of course they're not going to think, "I'll write a smelly version if I only have 3 days," because they are great programmers (aren't we all?) -- unless they track their time really closely and compare past work.
Paul Stephenson
As I said - not always easy! "Pick a number and double it" was advice I was once given.
Valerion
I like them "give a range" method, I originally saw it at http://www.codesqueeze.com/never-be-responsible-for-your-estimations-again/. Although I think instead of "% confidence" it should be a hybrid of "% confidence" and "% smelly"...
BryCoBat
+1  A: 

If the problem is the developers (and I've certainly seen cases where it is them, rather than the managers) the answer is probably something you've already mentioned: code reviews.

Code reviews should not be just an abstract exercise - if the code is smelly, then the reviewer should fail it. If the review fails then the developer should not be allowed to check it in.

It's also true that very tight deadlines will cause developers to check in hard-to-maintain code. But it's the manager's decision to save time short-term (take on technical debt) by doing that. If that's not what's wanted then the developers need to revise their estimates to make sure time is allotted for writing in maintainability.

P.S. you might have to work round some seniority issues. It can be hard for a junior developer to fail a senior developer's code, no matter how smelly it is.

DJClayworth
+2  A: 

I had a little "contest" posted on my office door/window with the results of developers who found and fixed undocumented defects or removed redundant lines of code or refactored. There was no official prize or management acknowledgment and only a few developers "participated". usually they did so while in the code fixing other things- leaving it all much nicer than when they went into the code.

I got in trouble for having the "contest". Apparently the people who were writing the smelly code were upset that others were fixing it... I am continually amazed at the poor developer and management we have in this industry. Oh well.

Tim