Fog Creek Software
Discussion Board




Managing and fixing spaghetti code

On the "what to do" thread I mentioned that I just started a new job with a large inherited code base in VBA.  It's a horrific mess.  My question is: How much time do I spend cleaning it up?

On the one hand, my employers don't care how ugly it looks under the hood so long as it works.  On the other hand, it's so entangled (e.g. globals out the proverbial wazoo) that I have to do at least some refactoring just to be able to modify it safely.

How do you handle this sort of thing?  Set aside a block of time for only refactoring?  Try to do it as you go along, balancing refactoring with visible results?  Change as little as possible and just focus on the new features?  Do companies recognize that when you're refactoring, you're working, or do they just wonder why the new widget they asked for three weeks ago isn't working yet?

Kyralessa
Thursday, August 12, 2004

+++Change as little as possible and just focus on the new features?+++

Do this where feasible, refactor as you go where it's not.  Most employers (yours may be miraculously different) want results, are very used to hearing (and not believing) "This guy's code is crap and it needs fixing", and consider you spending time re-doing what they perceive to be already done as wasting their investment in the initial codebase.  The latter can actually be a mix of them actually believing that you are throwing their investment away and them not wanting to admit to you (or to their management) that the last guy they hired really DID write shit code and his time with the company was wasted (and Manager A sucks at interviewing).

Good luck.

muppet
Thursday, August 12, 2004

Concern about management's perceived value of refactoring is very valid.

Especially when one considers that you already know they don't care what it looks like under the hood as long as it is functional.

Unless I was specifically asked to refactor (after of course suggesting that it needed it) if I were in your position...

I would wait until something was broken, or a change, enhancement, or some sort of other work on the system  was requested. I'd purposely overestimate the amount of time I thought it would take me to do such work and I'd use the excess to refactor.

Now I say all this due to the general lack of IT knowledge many of my direct supervisors have had. In such experiences, management not only does not care what something looks like under the hood, they may very well not want it pointed it out (and certainly not quantified with man-hours) either.

At any rate, don't expect a raise for your extraordinarily successful 'port' to something intelligible that still (to them) functions exactly the same.

Metric-Schmetric
Thursday, August 12, 2004

Precisely.

errr.. isn't that about what I said?  I name thee Redundant Man.

muppet
Thursday, August 12, 2004

I was busy refreshing to see if you responded to my rupees comment, so I couldn't type as fast as you.

duly confused
Thursday, August 12, 2004

and Kyralessa, this really does happen at nearly every programming job.  We recently had a Java contractor in for six weeks at twice my hourly salary to write a tool app I could have written (I even had TIME to write it, imagine!).  He was initially budgetted for 2 weeks (extremely optimistic), told to hurry the hell up at 4, and finally kicked to the street at 6.

It took me about 5 weeks to reverse engineer his code and decipher his cumbersome and poorly written documentation, and then basically completely refactor the app to make it actually meet the original specifications.  When the Director of our department asked me why it took 5 weeks to complete a "mostly completed" application, I told him precisely why, and cited (and displayed) examples, explaining them as best I could to a non-IT person.  My supervisor was not best pleased.  He would have preferred to paint me as incompetent and slow.

Anyway.. this crap happens everywhere.  Try to work with what's there as best you can, but when it comes down to it, do whichever will allow you to complete Task Y faster.  Don't worry about Task Z which hasn't come yet.

muppet
Thursday, August 12, 2004

Visible Results
Visible Results
Visible Results

Some refactoring where necessary


Here is how Joel approached fixing a broken process:

http://www.joelonsoftware.com/articles/fog0000000332.html

So I allocated the first seven hours of every day to just writing code, as was expected of me. There's nothing like a flurry of checkins to make you look good to the rest of the development team. But I reserved another hour every afternoon before going home to improving the process.

Andrew
Thursday, August 12, 2004

I'm not aware of an xUnit for VBA specifically, but there are some for plain 'ole VB. As I started to write new code, I'd make sure and do the unit tests first. Then as you have to make changes to the old code, write your unit tests first for the changes you're about to make.

And say nothing to your bosses about "refactoring" - just do it as it becomes necessary, i.e., you can't get your job done unless you refactor.

Michael Ealem
Thursday, August 12, 2004

The remark about testing is a heads-up.

Unless you can **guarantee** that your code changes do not break anything, and have no side effects, and will let you sleep nights, then do not change a line until authorised by your client, or your employer.

I've been tempted into cleaning up ugly code, and it's satisfying, like tidying desk drawers. But where to stop?

In bugfixing, the less you change the better, as the code delta is kept small. If you want to ready a platform for the next big version, make a case for tiding up and parallel testing a copy in isolation from production in your spare time at work. Just don't make it an unpaid sad hobby.

trollop
Thursday, August 12, 2004

Have read Fowler's _Refactoring_ book? It speaks to your concerns in its first 5 chapters. http://martinfowler.com/books.html#refactoring

Christopher Wells
Friday, August 13, 2004

What I've found useful in the past is to put together a report on the current state of the code and what's wrong with it. It's best to do this in a language that your intended audience understands... Once you have the problems listed you can provide details of why these are problems and what issues they could cause going forwards, again in the language of your audience. Then suggest changes that could be made and include details of the complexity of the change and the risk associated with making the change; link this to the risk associated with not making the change. Often the biggest risk of not making the changes may be an inability to reliably schedule future work (ie I've no idea how long it'll take to make a change because the code's such an interconnected mess of side-effects)...

Once you have all of this you can send it to the people in charge and see what they say. Chances are they'll 'accept the risk' of leaving the code as it is.

Going forward you can refer back to this document as and when the risks bite; you can also refer to it when suggesting that before doing feature X you deal with item A from the report due to the risks involved, etc.

If and when you DO get the goahead to make some changes I'd advise you to try and get some tests in place before you make the changes. The tests will help you prove that the changes you've made havent changed what the code does, just how it does it.

Always do refactoring changes and check in and then do feature changes or bug fixes. It makes the source control diffs easier to manage (for those that like such things).

Once you have some tests in place for some of the explicitly permitted refactorings you can use these as a crutch to make other changes. You could do these in any 'spare' time you have, on a branch and get approval before integrating.

Document what you do and the reasons for it and make people aware of the risks of a) doing the changes and b) not doing the changes.

I've written quite a lot about this kind of thing on my blog but it's mostly relating to C++ code; check the testing and refactoring categories.

Good luck ;)

Len Holgate (www.lenholgate.com)
Friday, August 13, 2004

i'm finally starting to feel a little better about my job, mainly because i finally see some ways to make things better.

I'm going to really pursue the automated unit testing, i'm looking at:
VBAUnit (it's on Wiki)
and http://www.amazon.com/exec/obidos/tg/detail/-/0782143202/qid=1092342636/sr=8-2/ref=sr_8_xs_ap_i2_xgl14/103-1610493-8722269?v=glance&s=books&n=507846

also, i'm slowly starting to change code. for example, i recently solved a bug where the business logic in the form was not the same as the stored procedure. obviously, i'm going to remove the business logic from the form. things like this, slowly and surely will make a difference.

finally, as mentioned our main app is Access/VBA/SQL Server. we have several SMALL custom apps/utilities, I'm going to rewrite these in .net. In addition, i'm looking at writing a small code generating app that will fix some of the spahetti code. i'll introduce nunit to the company.

what i'm doing is NOT earth shattering, it's not going to magically fix the real app and the real problems. But, slowly and surely i can introduce change. follow nike's advice - just do it!

Patrick
Friday, August 13, 2004

This is how code "ages" and why a 10 year old program becomes unmaintainable. The bad things accumulate, technical debt grows.

Essentially it is deferred maintenance, first you put duct tape on the cracked window, then you flush the toilet with a bucket you fill from the tub (which leaks so you have to sponge bath), and soon your lovely new house is a slum.

Some people say 20% of people's time should go to refactoring and trying new things out, that would be one day a week.

I was at a job, 15 year old product, company went out of business because no new following product.

With the 20% rule, since they usually had 3 to 4 programmers working, that would be 3 to 4 work years that would have been available for a web-based or Windows based version instead of Unix and ascii terminals.

But one day the orders stopped and then the existing customers dropped away one by one and now there is no more company.

frustrated
Friday, August 13, 2004

> How do you handle this sort of thing?

Personally, I refactor the classes that I must touch anyway in order to add a new feature or to fix a bug.

It's rare that I refactor for the sake of refactoring ... and even then it's in response to a management request, for example to move some functionality into a separate process that can be run on a remote machine.

> Set aside a block of time for only refactoring?

Given the above, yes: one task becomes two subtasks: first refactor (without changing functionality), and then add new features.

> Do companies recognize that when you're refactoring, you're working, or do they just wonder why the new widget they asked for three weeks ago isn't working yet?

If the code needs refactoring before the widget can be implemented, then management will hear from me that "the current code isn't designed to support the new widget, so coding the new widget is only half the job: 10 days to code the new widget, and 5 days to integrate the widget into existing code".

Christopher Wells
Friday, August 13, 2004

I'm not denying that there is a lot of bad spaghetti code out there: certainly I've seen my share of it.  But there also seems to be a tendency, particularly among more junior programmers, to want to throw out any legacy code and rewrite it rather than put in the time to understand it, using the excuse of "spaghetti" or "unmaintainable".

Many times (and I've felt the impulse myself, but now resist it) when facing a new set of code that has a long history, developers will feel "I can rewrite this faster than I can understand and fix the current code".  When they do, it takes a lot longer than they thought.  Then they get under pressure to finish what was originally a "simple bug fix" that has now been outstanding for weeks.  So they start taking shortcuts themselves.  They end up with new spaghetti code to replace the old.  *I've seen this happen*, more than once.

Some code is complex because the domain is complex.  The complexity may not be obvious at first -- it may have to handle a lot of rarely-occurring but critical alternate use cases.  Or it may have been patched by 10 different people over the years to handle new and changing requirements.

Dig in.  Understand what it's doing before you start changing things.  Certainly clean up/refactor code that is related to the problem you are trying to fix, but don't spend time cleaning up code that otherwise works to managements/the customer's satisfaction.

Oh, and one thing that *IS* worth doing -- add extensive comments to the code as you figure out what it does.  I do this obsessively and it always pays off.  In my experience and opinion, most developers do not comment nearly enough.

AMS
Friday, August 13, 2004

"used to hearing (and not believing) "This guy's code is crap and it needs fixing", and consider you spending time re-doing what they perceive to be already done as wasting their investment in the initial codebase. "

I'll play devil's advocate here:

How does the manager/owner know that refactoring WOULD be an improvement and that he would not, indeed, end up with code that is significantly better.  Now he's paid for programmer time to create something that only that programmer understands.

Then the next programmer comes along and says the same thing.

I'm not saying this IS the case. But, unless you can demonstrate that it is NOT, the owner has now good reason to do that.

Mr.Analogy (ISV owner)
Friday, August 13, 2004

Mr. Analogy -

really, that was part of my point.  You have to overcome the fact that Manager A has heard that his code is shite a million times, and why should he believe YOU?

muppet
Friday, August 13, 2004

> You have to overcome the fact that Manager A has heard that his code is shite a million times, and why should he believe YOU?

He can believe you: partly because he knows you work well and are trustworthy; partly because you work hard (hiding from him the cost of refactoring ... in your case for example, where you finish your day's assignment in two hours, you could spend the other 6 refactoring); and partly because you do it politely, for example "I can improve this code (which, after all, is what you hired me to do)", instead of "your code is shite".

Christopher Wells
Friday, August 13, 2004

When you tell some managers "I can improve this."  what they hear is "Your code is shit and you can't hire competent developers."

All the politeness in the world won't save you from pigheaded project managers.

muppet
Friday, August 13, 2004

AMS: Very code point. I think the advice to dig in and really understand it is key. I've seen the same kind of situations you have, but I've actually seen more that worked out well -- where the resulting code was more reliable and maintainable. I think learning to refactor and fix existing messes is a crucial skill, but you also need to be able to assess when to stop throwing good money after bad.

A separate point: If you work in a small shop where some of the managers were responsible for writing the original code that you want to replace, _tread_very_lightly! There are many people in this business that don't understand the long-term effects of continuing development (basic laws of thermodynamics in other words :) and will take offense at the mere notion that anything they did before is no longer servicable.

Jeff Kotula
Friday, August 13, 2004

> When you tell some managers "I can improve this."  what they hear is "Your code is shit and you can't hire competent developers."

That's unfortunate; because the message I intend is "You've found in me a competent developer".

> All the politeness in the world won't save you from pigheaded project managers.

There's some shared and some division of responsibility between developers and project managers. As a developer I like managers to:

*  Pay me
*  Say what their *minimum* requirement is, e.g. "I want you to implement and integrate widgetX within the next 3 weeks"
*  Give me permission to do whatever I think I need to do to accomplish that task
*  Give me access to resources (hardware, write-access to source code version control database, support from QA, ...) that I find necessary or desirable for this task
*  Let me do a good job

Sometimes I can understand (and agree with) why they may be reluctant to let me refactor (e.g. if I were no good at refactoring; if the code didn't "need" refactoring; if there were no test cases; if they need the job done "quickly" rather than "well"; if my editing source at this moment would interfere with someone else's modifying the same source at this moment; ...).

Usually though they don't need know that I am "refactoring", and have no reason to forbid me to do it.

Note that "refactoring" may be developer-speak instead of manager-speak, so if you say that you're "refactoring" they hear "wasting time and introducing risk turmoil, by changing something that one is not supposed to change, because one can't be bothered to understand the status quo" ... instead of "refactoring", "integrating" might be a more positive-sounding word that a manager will approve of.

Christopher Wells
Friday, August 13, 2004

"*  Give me permission to do whatever I think I need to do to accomplish that task"

Good luck on this one.  Most of my projects come with many (ill informed) limitations based on divisional politics.

muppet
Friday, August 13, 2004

> Most of my projects come with many (ill informed) limitations based on divisional politics.

I can imagine: good luck with that. For 12 of the last 16 years I've worked at a company where I can speak to the owner.

Christopher Wells
Friday, August 13, 2004

"But there also seems to be a tendency, particularly among more junior programmers, to want to throw out any legacy code and rewrite it..."
"...Some code is complex because the domain is complex.  The complexity may not be obvious at first..."

Right you are.  I daresay I was fortunate in this case because the domain and the existing system are both sufficiently complex that I haven't had thoughts of scrapping it and writing from scratch.  But actually even in my last job, where I did completely redesign a new database they were using, I continually used the old one as a reference point.

Actually it has crossed my mind to redo the whole thing, not because I want to wash my hands of the old one but because, seeing what the old system does, I think a new one could significantly simplify and improve things.  But I know that I have to make a business case for it; granted it may be better, but will it be so much better that it's worth them paying me until I've designed it?  In the meantime, time spent fixing the most pressing bugs and enhancements isn't time wasted because it adds to my understanding of the system.

Though I'm not sure I've treaded lightly enough, merely the fact that the last programmer came from the same recruiting firm as me was enough to tell me I ought to go easy on my opinions of his work; to say "that last programmer was terrible" also says "you were fools to have him on staff for a year and a half."

And as it turns out, this code has been through at least three different people, and the first was just someone with a VBA book and no programming experience.  So it's hard to tell which of the various programmers was responsible for which part of the code anyway.

Most of the refactoring I'm trying to do involves simplification.  Every time I remove a never-referenced global variable or move repeated code into a function, I'm making it easier for me to understand what the code does.

I guess the hardest thing is knowing how much time it _ought_ to be taking me to do what I'm doing vs. how much time it actually does take; am I working quickly enough, or too slowly?  I'm not sure how one judges this.

Kyralessa
Friday, August 13, 2004

Based on what you've said, 3 options:

1) Leave it as is (don't refactor while fixing/enhancing)
2) Refactor while fixing/enhancing
3) Rewrite from scratch

I'd say that 1) isn't a good option.

You can do 3) in a guilt-free way if the code doesn't work and has never worked.

I hope that you have or will read http://martinfowler.com/books.html#refactoring

I can't answer your last question ... instead of how long *should* it take, I'd say just "do it as fast/well as you can" ... that's all you can do ... it's your employer's option to hire someone else to do it if they want to, but meanwhile you only have you.

Christopher Wells
Friday, August 13, 2004

*  Recent Topics

*  Fog Creek Home