Should this code be rewritten or refactored?

Question

There is a module in our telecoms equipment which is written in C. I think the code in this module has a bad smell because it has a number of symptoms:

• When new features are added to this module, some original features go wrong. And some weird problems were finally orientated to the buffer overflow of this module.

• It is difficult to add new code to the module. Although the unit tests and integration tests covers new code, errors and flaw concealed in the original code come to the surface.

• By means of code review, we found the quality of the code is extremely bad. But the module was tested for many rounds, the errors concealed in the surface of the code were found by the QA engineers. Other problems hidden deeper were difficult to find. as well as a virus in the body.(easy to bring misunderstanding, delete it)

We summarize the status quo lay in these reasons:

• The original code has corrupted because of the loose process monitoring, in fact, many self testings are ignored or partial executed.

• The first programmer of this module was lack of the programming ability and the team leader didn't find the problem and risk in time.

• Many improvement approaches stay in the plan phase due to the team leader's suspicion.

The project came to a new version. Some team leaders want to change this situation and we discussed the procedure many times. Everyone has agreed to revise the module but we have to decide between:

• rewriting the module. It means throwing all the code of the module to the trash can and rewriting the code without changing its interface.
• refactoring the module. This means one step at a time. we need retain all the code at the beginning and build a solid set of tests for the code. Then we dig into the code and revise a small piece of it, and relying on the tests to tell us whether we introduce a bug. This process is repeated until the refactoring complete.

I agree with rewriting because we already have some rewriting experiences in our systems which were proven to be the best choices. The project manager wishes to refactor the module on account of the limited time available required to complete the detailed design, coding, code review, unit tests and integration testing which is three months. He thinks the time scales are too tight to do a full rewrite.

Should this code be rewritten or refactored?

EDIT:

As Joel said in Things You Should Never Do, Part I

The idea that new code is better than old is patently absurd.

I agree with this viewpoint. But I have some puzzles that some rewriting cases in our project proved to be wise choice. Maybe it relies on the new programmer who is more outstanding than the previous programmer or the testing is more sufficient.

Answer 1

That's easy.

Rewrite it.

Only say that you're refactoring. Everyone is happy.

Since neither word has a formal, legal, testable definition, they can argue the subtle semantics of each word while you rewrite/refactor until it works.

Answer 2

It doesn't sound like the module was bad to being with, but that adding "new features" broke it. At this point it may be hard to tell whether the code quality was low originally or that came in with the features.

Either the module is unmaintainable (a distinct possibility) or the people tasked with updating it don't have the right skills (sounds like this) or both.

Based on your misuse of the term "page fault" (page faults actually are normal, this sounds more like an access violation), your idea that a buffer overflow is a property of a large module of code rather than a bug in a specific line of code (and there may be many buffer overflow bugs in a module), and the fact that you seem to be using the word "virus" to refer to that buffer overflow, I'm afraid you're as much the problem as the code is.

At the very least, I think you should stay out of the "how to best fix it" debate and let more experienced coders work out the right approach, then help them accomplish it.

And then spend some time reading (blogs would be good) about writing robust code, how to avoid buffer overflows, integer wraparound, etc. so that you are better able to deal with such issues in the future.

Answer 3

Don't rewrite until you are sure that you really understand the code. When you aren't -- and it is the impression you give -- it is a bad idea. I started to explain why, but remembered that Joel did it better.

Answer 4

Refactoring a module is rewriting it, without changing its interface. So if you ask what would be best for you, I assume that you ask if it makes sense to change the interface or not.

Changing the interface could be a pain, integration wise. Anyway, if your module is used only in very few places it could be feasible a rewrite, because you know now much better than before what the module is supposed to do and how it is really used. This knowledge should help you design near optimal module architecture and API, possibly eliminating large amounts of intricacies.

Otherwise a refactoring is your best option. In your case the module should already expose some API, so you could start rewriting the bodies of those functions, while still honoring what they currently take and return, and how they fail. In this refactoring you should rely on still to be written functions, in a top-down fashion, for any needed results.

Answer 5

The project manager wishes to refactor the module on account of the limited time available required to complete the detailed design, coding, code review, unit tests and integration testing which is three months. He thinks the time scales are too tight to do a full rewrite.

I think your project manager has poor understanding of what to refactor means. What he is talking about is at very best patching the code. Refactoring doesn't imply in any way skipping any of the steps you listed (coding, reviewing, testing) and thus there is no guarantee that it will faster that a full rewrite, especially if the starting point is poorly written code.

I agree with some of the other respondants on the fact that refactoring vs rewriting is arguing about subtle semantic differences. The problem is that your project manager is probably thinking to neither of the two.

Answer 6

Other problems hidden deeper were difficult to find, as well as a virus in the body.

How did THAT happen?! Are you using version control?

Generally I am not an advocate of re-writing code from scratch since production code will usually have been modified over time to deal with a lot of nuances. That said, the whole "virus" thing would make me more inclined to start from scratch since I'd want to review it in microscopic detail anyway.

Answer 7

IMHO: rewrite as a last resort.

I did advocate for and get approved a project to rewrite our flagship product. Huge undertaking. I did this only after all my refactor attempts failed. The product had code beyond smelly, it was riddled with constructs creating undefined behavior. It could not be worked on effectively.

If you have any hope in refactoring, refactor. If there is none...don't. Unless the project is beyond trivial, it shouldn't be possible to rewrite in less time than it takes to refactor.

Answer 8

This made me think a lot: "When new features are added to this module, some original features go wrong." I have seen this happen quite often and, IMHO, the reason is mostly related to the fact that the "open/close" principle is not followed.

Basically, you should not add new features by modifying existing, working, and well-tested code. Rather, you should add new features by writing new code on which the old code does not depend.

So, if extending your code breaks old features, can it be that the code is not being extended properly?