58

The codebase I work with daily has no automated tests, inconsistent naming and tons of comments like "Why is this here?", "Not sure if this is needed" or "This method isn't named right" and the code is littered with "Changelogs" despite the fact we use source control. Suffice it to say, our codebase could use refactoring.

We always have tasks to fix bugs or add new features, so no time is put aside to refactor code to be better and more modular, and it doesn't seem to be a high priority.

How can I demonstrate the value of refactoring such that it gets added to our task lists? Is it worth it to just refactor as I go, asking for forgiveness rather than permission?

5
  • Institute code-reviews and the problem will take care of itself (almost) Aug 23, 2012 at 11:44
  • 4
    Don't treat refactoring as a separate task - it's not.
    – Vetle
    Aug 24, 2012 at 11:36
  • 2
    In-code changelogs make me want to cry... I'm sorry for your loss. Aug 24, 2012 at 18:58
  • @Mark Canlas I used to think the same way until I encountered a 20 year old code base with literally hundreds of changes in source control. Good luck finding why (or even if) a particular code block was changes using only source control history
    – Michael J.
    Feb 20, 2019 at 22:07
  • @Michael What made it so difficult? In general, a few blame/annotate operations should get you right to the relevant commit no matter how many changes were made. (“Hundreds” of changes over 20 years is tiny, BTW.) Mar 26, 2019 at 4:12

17 Answers 17

51

"It's better to ask forgiveness than permission" is true.

Why worry about it? Just refactor the most horrible parts.

You already know what the most costly errors are, right?

If you don't, then step 1 is to positively and unambiguously define the most costly, complex, error-ridden, bug-infested problem code.

Identify the number of trouble tickets, hours of debugging, and other very specific, very measurable costs.

Then fix something on that list of high cost problems.

When you have to ask forgiveness, you can point to cost reductions.


In case you aren't aware, refactoring requires unit tests to prove that the before-and-after behaviors match. Ideally, this should be an automated, coded test, for example, a unit test.

This means pick one thing. Write a unit test. Fix that one thing. You've made two improvements. (1) wrote a test and (2) fixed the code.

Iterate.

11
  • 4
    If you do this, get metrics before you start, then when asking forgiveness, you have the evidence you will need to keep your job.
    – mattnz
    May 17, 2011 at 21:02
  • 15
    "Why worry about it? Just refactor the most horrible parts." This will be very dangerous advice without unit tests. In combination with your other advice to ask forgiveness rather than permission, OP may be asking forgiveness in a big way. Just wait until a ticket is opened because of unintended behavior change that can be tracked back to unauthorized refactoring. There is a good chance it will be blamed on the practice of refactoring rather than the lack of unit tests, and then this will serve as eternal "proof" in this office that "refactoring is evil". May 18, 2011 at 12:43
  • 4
    Also, I have rarely ever seen a company that made use of unit tests, so how do you even begin to refactor in a situation like that? This seems to be a self-defeating downward spiral: You can't refactor because you have no tests, you can't write tests because the codebase is too large and/or there's no time outside of writing new features to go back and write tests for code that has been written for years, so you cannot ever refactor anything, so the code just bloats and rots until it collapses. May 18, 2011 at 13:38
  • 8
    Most of the time the answer seems to be "Leave and find competent people who understand how to be a real professional, not a hack." :) May 18, 2011 at 13:47
  • 4
    At some point horrible or hard-to-maintain code is itself a bug. Create backlog items and assign them priority. I work in an agile environment where we do a short stabilization sprint from time to time when the client is being too airy to give us specifics or is out on training. We don't stop because they are unavailable. And when the next sprint starts, we've had time to familiarize ourselves with each others' contributions and be more accurate in our effort estimates. You just have to start somewhere, even small, and keep going. Don't make it worse by continuing the style while you're at it.
    – Erik Noren
    May 18, 2011 at 19:36
33

Follow the boy scout rule: leave the campsite (code) a little better than you found it. I have never once heard of someone getting written up for making small code improvements "while they were in there."

5
  • 7
    Strongly depends on how close you are to a deadline. Changing a library may invalidate all previous testing.
    – user1249
    May 17, 2011 at 21:01
  • 3
    Good point, @Thorbjørn. Something like that I probably wouldn't classify as a "small" improvement because it has a large scope of influence. May 17, 2011 at 21:42
  • if you just happen to pass by a function which can be improved a bit. You just don't know that it is placed in a common library?
    – user1249
    May 18, 2011 at 17:42
  • @Thorbjørn I'd say you should have a good idea of where the function is being used. If the given source file is being compiled for something internal, i.e. so you have complete access to all its callers, I don't see a problem with fixing it and updating the places where it's called as needed. If the source file is being compiled as part of a library where the API can't be changed, you could at least fix implementation details.
    – Maxpm
    Jul 13, 2011 at 8:38
  • I've seen the kind of "this needs to be refactored" comments sneak in on code that is reused in other places but where it's not very clear which ones those are. Usually the developer doesn't want to spend the time to do impact analysis, and they put in the comment to assuage their guilt. Aug 14, 2012 at 11:16
24

I'll take a perhaps overly cynical point of view.

Refactoring is such a hard pill to swallow. You get the responsibility and the blame, and the fruits of your labor often goes to someone else who builds on your clean code base.

I would say that if such engineering practices have become the culture at your company, you might need to fight at a higher level. You're not really fighting for refactoring, you're fighting for engineering excellence, and that's the kind of change that only dawns on management when it smacks them in the face. In that scenario, they are probably going to look outside for a best practices czar, and all your great ideas will be subsumed anyways.

Consider joining a different company where they take engineering practices more seriously.

2
  • 2
    My experience is that Engineering excellence rarely pays the bills. Its a balancing act that only a few programmers are good at. Provided the OP is not striving for over-engineering excellence your comments are valid.
    – mattnz
    May 18, 2011 at 3:32
  • 8
    This is almost always the best advice IMO. If the company doesn't understand why having quality code is something that should be encouraged and not seen as a waste of time, it's a lost endeavor - they aren't intelligent enough to understand real programming. May 18, 2011 at 12:09
19
+100

I notice that many of the posters here seem to be convinced that the problem in management - while the question doesn't mention that.

I'd go further than that: in my opinion a lousy code base is almost never directly the fault of management. Management did not write that code, the developers did (there are some exceptions at my company where some of our current management actually wrote the initial code base). Therefore the culture problem resides with the developers - if they want the culture to change, they themselves will have to change.

I try to bring this realization and change of attitude across to "my" developers as well. Whenever they ask me "when will we have time to refactor?", I act surprised and reply "you should already be refactoring all the time!". The only way I believe you can keep a code base healthy is three-fold:

  1. Only ever add healthy code.
  2. Always fix not-so-healthy code as soon as you see it.
  3. If for deadline reasons you cannot do 1 or 2, always have a list of "fix immediately after the deadline" issues, and do not accept any excuses for not working through that list after the deadline.


Invariably the next remark from developers is "but how do we get time to do this - we don't have the time now?". The only correct answer (again IMHO) is "you don't have the time NOT to do this.". If you don't keep the codebase healthy you will find turnaround time getting longer and longer, schedules getting more unpredictable, bugs getting nastier, and value getting lower.

The biggest attitude change you need to effect in the development team is that "quality" is not something you do at a specific moment in time ("when we have time to refactor") - it's something you have to be doing ALL the time.

Finally, a story of warning. If pressed, I will deny this ever happened. At a company I worked for there was a long-standing application with a large, legacy codebase originating over 10 years back. Lots of developers including me believed this legacy codebase was bad or at least outdated, not state-of-the-art. So we lobbied for a big refactoring project successfully, and started the rewrite project after which we believed everything would be better.
We worked long and hard implementing almost everything in a new and modern way, using new libraries, new language features. Near the end we put in a massive effort to bring everything together to allow a new release of the long-standing application with the new and improved code-base.
As expected the new release had some teething problems, because of the new libraries that we were not yet so familiar with, and some interactions in our code that we had not foreseen. However, we finally managed to get the release to the same standard as our previous releases and out the door. We breathed a sigh of relief at our "success". Then a sub-group of developers went back to management, asking for a new refactoring project because our new code wasn't quite the silver bullet they had expected it to be, and they saw some opportunities to completely rewrite some things...

Moral of the story: often things aren't nearly as broken as they seem, and 'starting over' usually means you will trade a known set of issues with a at-least-as-hairy unknown set of issues. Refactor one part at a time!

6
  • 2
    I think this is a case for modularization, as I mentioned in my response, IMO this can tackle some of the issues by breaking things down into smaller apps, if possible, then you can rewrite one part of it every week if you want while leaving the stable modules alone Oct 3, 2011 at 2:01
  • This article is very apt: blog.objectmentor.com/articles/2009/01/09/… Aug 23, 2012 at 16:31
  • See also the Things you should never do from Joel Spolsky.
    – Jan Hudec
    Aug 24, 2012 at 11:29
  • I am all for the advice that "you don't have time to NOT refactor." But to claim that this is the developer's problem? Are you kidding me? I can't tell you how many times management (non-programmers) has literally called me in to the office to tell me to stop refactoring, leave the code in the most recent version, and quickly do something else. We have several developers constantly arguing that we should do refactoring, but management literally does not value it. This is common when managers are technical workers in some domain other than software.
    – user103181
    Jan 31, 2014 at 15:04
  • Well, from my experience this is always a management problem. Management is there to manage people so they do their jobs properly. If the programmers don't do their jobs properly (and not writing quality code means exactly that), then we have a hell of a problem in management!
    – Kaiserludi
    Jun 12, 2014 at 12:20
11

Someone once told me that when I go to an interview, I should not forget that I am also interviewing the company. Is it a place I want to work? Do they do code reviews? Do they have automated integration tests? Unit tests? What do they think of pair programming? Personally I'd find another job, and don't forget to ask some questions too this time.

4
  • Good advice, but asking questions doesn't always work. I've interviewed at some companies that lied about that stuff - e.g. saying they use version control but it's not set up right at all and nobody really knows how to use it anyway, saying they do testing but there are no unit tests, saying they use the latest and greatest technology but aren't actually using any of the features in the newest version (or any version past the first). May 18, 2011 at 12:08
  • 6
    @Wayne M: In that case, start looking for a new job immediately. If they lied to you in the interview, how are they going to treat you later? May 18, 2011 at 14:20
  • 1
    Agreed, but sadly often easier said than done. May 18, 2011 at 14:54
  • @WayneM Exactly. I've experienced the same thing. I asked about creative opportunities to do math research in a job, and the company basically lied about it to get me to accept and then stuck me with projects I had thought that I weeded out by asking during interviews. The advice "look for a new job" falls pretty flat -- of course I will do that, it just doesn't represent any kind of "solution" to this issue.
    – user103181
    Jan 31, 2014 at 15:06
6

Find another company, honestly. Such improvements to the development process require huge cultural leaps that it will take significant time before everyone gets on the same page and by then you won't be caring as much.

If you feel like you still have some fight in you and haven't gone under yet, then make a final push. Try to get as much support from like-minded team members, disclose your exhaustion to superiors that do care about your well-being, outright bypass and distance anyone who might oppose your beliefs and try to push in some mandatory refactoring hours into planning for new projects/features.

If your passionate about what you do and care about your company, that would be a commendable effort on your side. If it's unappreciated, then respect yourself and bail out before you turn into a hollow programmer.

5

If I had to introduce one practice to make things better in this kind of context, it would be code reviews. Code reviews are

  • generally intuitively accepted by developers as a factor for better code, less bugs, etc.
  • collaborative and democratic
  • not too time-consuming if you timebox them properly
  • a good place to do refactoring if you don't have time to do so during "normal" development
  • a pretty effective Trojan horse to gradually introduce all kinds of best practices in terms of code design, unit testing...

You don't have to do code reviews systematically, only when committing large/complex code portions at first.

Of course if you feel you need official endorsement before introducing code reviews, you may have to convince your boss first that the code base is likely to collapse if things are left as they are.

3
  • 2
    That assumes the others know good development practices in the first place. I had a job once where the other team members didn't even know why my way was more effective; my well-written code that followed SOLID and applied design patterns was actually rejected in a "code review" because it was different and the senior developer didn't understand it compared to the rest of the team's style of just using code-behind events and the App_Code/ folder. May 18, 2011 at 12:29
  • Oftentimes you can solve such difficulties by asking people to just try your way and see for themselves if it works. If they refuse to do so or still don't see the benefits, I have to admit it's probably time to quit. May 18, 2011 at 13:08
  • 1
    I was once told my way was "fancier" but I had to field the complaint that it was harder to understand. The other way FWIW was copying a 300 line file, changing two lines, and committing. The justification for copy/paste in that case (and usually in my experience) is "that way you know you didn't break anything".
    – Kevin
    May 19, 2011 at 4:02
4

Here is what I do in such situations (in the 15 years of my career as a developer I have come across such code almost every day)

  1. Lead by example - I make sure to re-factor the code I touch. I ruthlessly delete old commented out code and large paragraphs of comments.
  2. Ask for re-factoring every time I am asked to review a code change, without which I do not approve the code review.
  3. Slowly people see the change, when the code become leaner, more readable and thereby less buggy. It takes time but slowly the whole team appreciates and adopts the practice.

The management never sets aside time for re-factoring code (they never have enough resources!), so doing it slowly and steadily is a right approach.

1
  • I don't mind even if one to two bugs creep in while during code re-factoring, such defect are caught and fixed much faster and easily in cleaner/leaner code!
    – Curious
    Aug 29, 2012 at 10:29
3

Have management do some research on "technical debt". Also refer them to the "Broken Window Theory", both of these effects have direct impact on efficiency, quality and morale.

"A clean worksite is a safe and productive worksite", and every contribution to a mess compounds the mess in a exponential fashion, not a linear one.

If the situation is not dealt with, eventually you will cross a point of no return, where it becomes economically infeasible, by dealing with it incrementally before that happens the benefits will rebound, giving you more fuel to deal with the problem as you go.

3

It sounds like your problems are more general.
The refactoring issue is both a symptom and a potential relief from part of the problem.

The Software Lead and the Team Allocate the Team's Time

From my experience, I think you may be encountering a problem that I call, "everybody's a software manager." Product managers, project managers and sometimes system engineers and testers can be notorious for trying to micro-manage developers who probably already have an experienced software manager. You may even have a few members on your team who believe their role is to manage.

If you are the software manager, make assignments for the refactoring you want, or better yet, have your team propose refactoring to you for your approval. So as to not micromanage, you might have guidelines about age/author/size/context of code to be refactored that can be freely refactored vs. needing approval. If a member of your team wants to massively refactor four big classes of complex old code he didn't write that are not part of his feature, his two week diversion is your problem, so you need a chance to say no.

You can sneak around, but I think it is better to just build your estimates carefully with time for analysis, design, coding, multiple forms of testing (at least unit and integration), refactoring, and risk as judged historically and by the lack of experience or clarity associated with the task. If you have been too open about the workings of your team (or have members on your team who are), it might be wise to narrow communications channels so they go through you and discuss resources and results, not methods.

Early Project Choices Create a Vicious Cycle for Refactoring

Software maintenance is hard. It is doubly hard if others in the organization are making choices at your expense. This is wrong, but it is not new. It has been addressed by Barry Boehm who is one of our great software writers who puts forward a management model he describes as Theory W.

http://csse.usc.edu/csse/TECHRPTS/1989/usccse89-500/usccse89-500.pdf

Often software developers are hammered to produce under the Theory-X management approach that says that workers are basically lazy and will not perform unless pounded into submission. Boehm summarizes and contrasts his proposed model as follows:

"Rather than characterizing a manager as an autocrat (Theory X), a coach (Theory Y), or a facilitator (Theory Z), Theory W characterizes a manager’s primary role as a negotiator between his various constituencies, and a packager of project solutions with win conditions for all parties. Beyond this, the manager is also a goal-setter, a monitor of progress towards goals, and an activist in seeking out day-to-day win-lose or lose-lose project conflicts, confronting them, and changing them into win-win situations. "

Quick and Dirty is often Just Dirty

Boehm goes on to point out the reason things are so miserable for developers on the maintenance team.

"Building a quick and sloppy product may be a low-cost, near-term “win” for the software developer and customer, but it will be a ‘‘lose’’ for the user and the maintainer." Please note that in Boehm's model, the customer is more of a contract administrator instead of an end user. In most companies, think of the product manager as a customer surrogate, or perhaps the person who buys the product for its feature list.

My solution would be to not release the original development team (or at least the original lead) until the code is refactored to at least meet coding standards.

For customer , I think it is reasonable to count the product manager as a customer surrogate, and the group of people rewarded for delivering something quick and dirty can certainly be expanded, so there is a big constituency for doing things the wrong way.

Refactoring is not Negotiable

Please don't back down from your role as a software manager. You should have authority and discretion to use your team's time in process and product improvements. In that role, you may need to negotiate your choices to make your team more professional. However, with regard to process, don't negotiate with marketing, because in my experience, that is a losing game. Negotiate with engineering management. It shows you have vision. Building a professional software team is an extension of their role, and is much more likely to be seen as a win-win.

2

You could always just wait it out. Eventually, the team will miss enough deadlines, and produce buggy-enough software, that management will throw up its hands and say that by God something had better change!

Okay, that's a risky proposition. But it's actually what happened at our shop several years ago (part of the difficulty was in communicating with management about deadlines, but that's another story), and is much of the reason we now have tens of thousands of automated tests, shared code ownership, the freedom to refactor, the willingness to delete dead code, and the highest-quality releases we've ever had.

Perhaps most surprisingly, nobody lost their job in the process -- something I credit to our boss coming to bat for us, and a consultant who argued the case for refactoring and continuous integration on our behalf. It always sounds more convincing when it's somebody from the outside saying it.

1
  • Agreed, but in the OP's case it really sounds like he's working with a bunch of incompetent hacks, so when everything comes crashing down around them it will never sink in it's because they didn't refactor the crappy code, because if they could understand that the code wouldn't be as bad as it sounds like it is. Real developers know the benefits of refactoring from the start and take the steps to do it from the get-go. May 18, 2011 at 15:22
1

I think answer to how to find time depends, on why do you want to refactor code.

If it works, there is no need for special refactor and you can do it when you touch that part of code. Therefore you don't need special time for that.

If it slows your team development you need talk to team leader about that and create special task for refactoring and that you will have time.

Same for run speed and other cases, if refactor can improve something and not only "good code looking" or your opinion about, how code should look like, and provide real benefit, create task or talk with someone that is responsible for that.

1

I laughed a little bit at the way you described things as that sounds pretty similar to the code base I work on so I think our situations are pretty similar. Luckily, in my situation I have a forward-thinking manager that has decided the best way to making the codebase better is through modularization using a modern web development framework rather than to just refactor the entire codebase. This way the troublesome spots of the main application can be re-written as separate modules and then be integrated into the main app (but still be essentially independent). This may be an approach you want to bring up since it wouldn't require refactoring the entire (presumabley large?) codebase you are working with.

Of course, I may be a bit off from what you are saying as perhaps your code base isn't as bad as the one I work with, in that case I would say why not do little optimizations as you go along? The dev's on my team have no problem removing stupid or outdated comments and things of that nature and I don't think thats something that should require input from management as usually developers are given some empowerment to optimize things as needed.

If the code base is really fragile then you need to be careful and this can be why they may not want to pursue major refactoring as this could end up turning into a months long project and would probably require branching of a project and putting dev's on that project and taking away from other development tasks, like immediate bug fixes that may cause them to lose customers, etc

All things considered, as far as other people saying you should quit, etc, I think it depends on how management sees things, if they understand the situation and realize why some things may take longer than they optimally should, then things could be fine as far as the work environment, but if they are constantly getting on you about a backlog of work, that could become detrimental over time. I'm fortunate to have management that basically realize the application is a piece of crap, but it does have a lot of customers and brings in money, so its still valuable and worth making bug fixes on, even if just for the short-term.

1

Your main problem is that the codes isn't getting enough visibility.

I suggets using a continuous integration tool like Jenkins, and a static code analisys tool integrated to it that measures cyclomatic complexity, naming standards, code length, etc.

When a programmer commitS a change, Jenkins will run the units tests, run the static code analisys tool on it and generate a web report that everybody can see, with traffic-light-like color status.

When the code quality is visible to everyone ( specially the team lider and the boss ) and version control and unit tests are there to have your back... people feel encouraged to refactor.

1

The code got this way slowly over many small changes. It will need to fixed that way too.

You can do this as you go along, first of all - increase all estimates by 10% to allow for code enhancement and long term maintenance. If anyone complains ask them if it better to check the oil in a car engine every week or to wait until the engine completely locks up.

Have a meeting and determine consistant coding standards.

Introduce basic rules to use as you go along:

Whenever new code is introduced into a class automated tests have to be written to prove the class works (not just the new code).

Whenever a bug is fixed automated tests have to be written to prove the class works (not just the fixed code).

Whenever a programmer modifies a file he has to fix all of the compiler warnings in that file and update code so it meets the new coding standards.

After a while the most used code will be up to date and be covered by automated tests. The older code will be updated when it is changed, if it is never changed then you never need to worry about it.

The important thing is to build these habbits into the standard coding tasks so none of them take a huge amount of time away from 'real' work but they all provide real benefits. Do not try to make a project out of refactoring old code, it is a horrific, boring, fiddly pain which will look like a lot of time wasting to non technies!

0

The way to get the resources (time) you need is to focus on aligning capability and desire. Your boss is driven by targets (typically profits or delivery times for new features), and he sees refactoring as engineers wasting time and eating into those targets. You need to find a way to convince him that the targets will be met and exceeded if you spend time refactoring.

So first step is to find out what pain you boss is feeling. Identify what his biggest problems are. Then work out how what you want to do aligns with fixing his problems, and present a strong business case for doing it. Use you defect tracking and project planning systems (time overruns) to provide evidence of where problems lie. You need facts, not feelings. Without metrics (bug counts / module, cost to fix these), refactoring is just a programmer having a play at someones expense. Your boss will only give you the required resources if you can show a strong business case for doing it.

Crap code is more often than not too expensive to fix. With no automated regression tests, the cost of testing refactored code is extremely high.

Most times I have seen bad code in real need of refactoring, there been a large number of undocumented features and complexity in the requirements that cannot be understood from the start. It's not a minor undertaking and my experience is an order of magnitude harder to estimate the cost of a refactoring job than add a new feature.

I would refrain from going ahead and doing it behind you bosses back (If it wasn't broken, and you you broke it, how does that look) - fix code that needs changing anyway, but if it ain't broke, don't fix it.

9
  • 2
    As to keeping sanity - you need to understand what motivates people in the organization. Once you understand little things, like boss does not care what the code looks like, it become easier. If that does not work, change jobs or get involved in a open source project and refactor all you like.
    – mattnz
    May 17, 2011 at 21:04
  • 3
    "If it ain't broke, don't fix it" is the single worst saying in our entire field, and needs to be removed entirely from verbiage. It fosters slothful behavior and encourages a culture of hacking things together as opposed to doing it right, and by association prevents it from ever being done right in the future. That attitude is a cancer. May 18, 2011 at 12:12
  • 1
    See my above comment, that's why. May 18, 2011 at 12:57
  • 2
    @Wayne M: I don't think mattnz is saying "don't fix it, ever", I think what he's saying is "don't fix it unless it is good for the organization and you can build support" which is much different and quite reasonable, IMO. May 18, 2011 at 16:47
  • 3
    @Wayne M: well said! The saying "if it ain't broke, don't fix it" and the word "refactoring" simply do not fit together. The very essence of refactoring is that software development is simply not a black and white world where "broke" and "fixed" are absolutes. May 21, 2011 at 13:27
0

Strangely no one mentions this:

To make things a priority, make them easy: Get a good refactoring tool.

There are excellent refactoring tools out there (at least for .NET afaik). Oh, and don't forget to write unit tests beforehand (as others already pointed out).

Good luck!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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