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).

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?

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?"

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.

> 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.

The other thing that needs to be considered is the total effort for the refactoring including necessary testing.

As a development manager the thing that scares me isn't the small fix, it's that it takes place on a critical much used library that has potentially far reaching consequences.

I had experience with multiple such examples. At the same time I was developing new functionality, my boss was also tweaking the libraries. So our tester kept telling me my code was broken. Then I'd look into it and find that the Strategy object used to calculate quantities in the whole app was broken. Again

Have you ever heard of unit tests? It's the only solution.

Unit tests, of course, are another hard sell. "what you need to spend just as much time writing tests as you did writing the code??"

But I find the benefits are almost immediate, at least for new code. It's not some abstract thing down the road - it's an immediate boost to dev speed.

I agree that producing clean code is part of our charter, but sometimes we're put in situations with close deadlines and a boss with no understanding of the importance of clean, organized and sustainable code. For example, I had a boss which would regularly check on my progress and wanted to see what I was working on and he'd say "that looks good, let's wrap it up by today", my typical response would be "it's not done, still a lot of rewriting a clean up to do" to which he'd respond "does it work? I don't want to spend much more time on this". I would then try to work really fast to clean up my code and finish up the functionality. I was forced to push through stuff that wasn't on par with my standards.

I learned how to deal with this by hiding my process. I wouldn't show all of what I had. This made things easier. I still met deadlines and I felt much better about the code.

Agree completely. Also the same point of other sources:

From 37 Signals' Getting Real[1]: You need people who are passionate about what they do. People who care about their craft — and actually think of it as a craft

And from Software Craftmanship Manifesto[2] :Not only working software, but also well-crafted software Not only responding to change, but also steadily adding value Not only individuals and interactions, but also a community of professionals Not only customer collaboration, but also productive partnerships

[1]http://gettingreal.37signals.com/ch16_Start_Your_Engines.php

[2]http://manifesto.softwarecraftsmanship.org/

Excellent points to add to this discussion. One must always weigh the time it would take clean things up.

I think the main point I was trying to make with my post was; a lot of times it would take less time to just fix things than it would be to ask for permission to do so.

The corrollary of this is that it almost always takes longer to explain to a Project Manager why something isn't done yet/can't be done/shouldn't be done that way (etc) than it would do just to finish it or do it another way or do it properly (as the case may be).

I mostly agree, however I think it is important to highlight that these considerations have a real-world impact.

One of the biggest fallacies out there is that good software engineering is about aesthetics and somehow detached from the resultant product, e.g. 'the user doesn't care about how it's implemented' etc. - this is patently false, as code quality dictates robustness, maintainability and ability to make changes to the code base, both during development and after shipping the thing.