Hacker News Reader

Comment by theoj | Hacker News Reader

Then again, refactoring without permission could be a really bad idea. You need to ask yourself a few questions before you proceed.

Do you have thorough unit tests for the code that you are trying to refactor? If not, be aware that there is no way to know for sure that your refactoring won't break the functionality of the code.

Suppose it breaks the code. Have you thought about the operational impact to clients and the financial costs?

Let's say the costs are low. How big and political is your organization? What kind of trouble will you find yourself in? As the hysteria rises, will you be fed to the dogs over this?

How bureaucratic is your company and how many people do you need to interact with to fix a functionality breakage? The more people you will need to interact with, the more damage you will do to yourself and your reputation. Others will resent working in panic mode to clean up after you (now widely known as the "rogue" programmer).

View on HN

Not to mention if there are other people modifying the same area of code in a parallel project. They won't have tender words for you when they try to merge back their changes and find it has completely changed.

Cleaning up code is a good idea but you need to be aware of the context. Always do the best you can.

+1. My most common conundrum these days is whether to favor improved quality or consistency. If I make improvements to the current project, how can I make sure they make their way into other, parallel projects that share code and architecture. Is it better to be "better" or "worse but consistent?"

> If not, be aware that there is no way to know for sure that your refactoring won't break the functionality of the code

That's not true. Some refactoring are safe and you can prove it, see http://www.refactoring.com/catalog/reverseConditional.html

Any refactoring party on some real messy code should consist of a long chain of little proven-safe logical steps like the one above, or pure renaming, or move-around. In the end, like when the sea is gone, you may find some smelly dead animals that you'll have to take bare hands and fix, or not (sometime it is simpy too heavy to move, you'll have to leave it there, under a thick isolation layer).

Very fair considerations. I think it is easy, as a programmer, to think of refactoring only in terms of cost, and to ignore risk. That is a mistake.

My own rules to keep myself honest in that regard are these:

- Never rewrite anything you don't completely understand.

- Never rewrite anything beyond what you would have had to touch to get the job done anyway.

- Never rewrite anything just because it bugs you. Have a compelling, practical, immediate reason why fixing it will save you time right now.

And my way of work certainly reflects my environment. I do a lot of work on code for which there aren't test cases, aren't requirements, aren't other developers looking at it, and isn't even a very high probability that it worked as intended the first time. In some cases, I'm even the sole arbiter of what "works right" means.

Certainly, the higher the quality of the code, and the more other people--developers or otherwise--depend on it, the more lightly one should tread.

Do you have thorough unit tests for the code that you are trying to refactor? If not, be aware that there is no way to know for sure that your refactoring won't break the functionality of the code.

If there are no unit tests ... the original coder didn't care about correctness, and refactoring towards testability is a good thing.

The more people you will need to interact with, the more damage you will do to yourself and your reputation. Others will resent working in panic mode to clean up after you (now widely known as the "rogue" programmer).

I'm in two minds about this. Yes, you will need to be careful about not introducing changes, and yes, occasionally fixes will be needed. But better code should be a plus to reputation. If it's not a net plus, be more cautious. If that can't be done, I'd be thinking that I was in the wrong job, and my reputation with those co-workers was irrelevant.

I totally agree. I recently wasted a whole month refactoring some code (not without permission) whilst working on a defect so that the 4 main UI's of our product had roughly the same code structure -- even if that code isn't actually shared between them it makes things infinitely easier if they behave similarly. Unfortunately, although the refectoring fixed the defect and allowed us to progress with some features, it broke lots of other vital functions and after 3weeks I had to make the call to revert back to the original.

My new plan to fix this massive code smell is to leave the company :(

I hear you but all this means is your refactoring wasn't good enough - maybe you underestimated the scale of the undertaking. I have been there.

Don't lose faith - just document all the features, ideally with unit tests, before you make the next fix attempt. Leaving the company is of course a solution too, I'd just offer them to take even more time out to really fix that code. I do agree that living with the bad code is not acceptable though.

I think it is worth making the distinction between refactoring and rewriting. I tend to take the strict view that refactoring should not change behaviour, so for example if a unit test fails, and you refactor some code, the unit test should still fail because the code should still be doing the same thing.

Since you say that you were fixing a bug, and the length of time it took, my guess is that what you were doing was a rewrite, not a refactor.

Someone else had a good line about refactoring, though I don't remember who it was: if you're refactoring code, and it isn't unit tested, you are not refactoring. You're just changing shit.

I'm facing a situation where there's a part of some code that I'd like to improve: and there are design flaws in the old code that will make it more than just a simple refactoring.

Although the subsystem is a small part of the system, it's used in many different places, and it's complex enough that I know that functionality will be broken. It's the kind of system that would really benefit from TDD but it's not practical to cover all the code that would be affected. There would be no substitute for a thorough round of acceptance testing and whack-a-mole trolling for bugs.

So my manager and I have to make a call about the benefits and risks of this work, which in turn connects to project management issues. I wouldn't want to do this late in the schedule, but would feel more comfortable doing it early. At certain times I might be able to do this work and have very little impact on other developers but at other times it could impair their ability to do work. (Yes, intelligent use of version control could help a lot here, but that takes coordination with the team.)

The point is that it's a team effort. There are some cleanups which are easy and low risk and you should go right ahead and do. Yet, all programmers have a way of underestimating just how much a twisty piece of code that they don't understand does, so you've got to work with your manager and the rest of your team to control risk.

These considerations apply to a much lesser extent if you're at a shop that uses version control.

Only if you're extremely confident adequate testing is in place – in any form, be it unit, smoke, user or whatever – to catch any potential regressions. Using version control does you no good if you don't catch the regression and roll back before customers are affected.

Depends how costly live bugs are

Are there really still shops that don't? It's not like it costs money.

Oh heck yeah. We at HN are immersed in tech. Our experience is with companies whose core domain is tech and take things like source control for granted. But what about your local gym or sports franchise or restaurant chain or newspaper? There are millions of companies that don't have any kind of technical director or direction. Where some manager thought it would be a good idea to have a web site, and hired a programmer who's measured on how fast he can throw it together. What's a testing server? Heck, what's a server?

Reply on news.ycombinator.com