7

I have a project at work that was written entirely by a scientist who was learning programming while writing it. It is almost 200,000 lines of C++, and almost every variable is a global variable (over 2,000 global variables). I think he found out about local variables about half way through writing the project. The few local variable names are almost always x, xx, i, ii, j, jj, M, or some other single letter name. This program is prone to seg faulting and running valgrind reveals nearly a thousand instances of memory corruption. It is totally reliant on undefined behavior, to the point where it only produces the correct results on CentOS 7, and Ubuntu yields completely different and incorrect results (it's scientific code). It is completely indecipherable by anyone but the original author. We are in a unique position now where the company is going to bet everything on this one piece of software written by someone who has never written production software before. There is an extreme bus factor here, because after working on this codebase for months, I'm baffled by almost every line, and implementing the most basic features takes an incredibly long time. No other developer at the company wants to touch this code. This is by far the worst code I've ever seen. I never saw code this bad even from freshman CS students.

Given this situation, is this an appropriate case to "burn it down, and start from scratch"? What is a good strategy to make this a maintainable code base? This code will need frequent updates as research progresses, meaning freezing the code is not an option.

For clarification, this project is not yet being used in production. It has been entirely proof of concept until now, and will be used in production this summer.

For anyone curious of what this would look like, the functions look something like this.

void doSomething(void) {
  sideEffect1();
  sideEffect2();
  sideEffect3();
  ...
  sideEffect145();
}

void sideEffect1(void) {
  if (globalVar1) {
    return;
  }
  anotherSideEffect1();
  if (globalVar2) {
    globalVar1 = globalVar2 + 1;  
  }
  ... hundreds of lines later
}
Kyle
  • 79
  • 3
  • Is the original developer still available to decipher the code? This would heavily influence my decision making. The only thing you said in the question that was scary (rather than just sad) was, "will be used in production this summer". I can only hope not. – andy mango Feb 18 '20 at 02:40
  • The original author is still at the company, and I've been given carte blanche from management to do anything necessary to get this codebase production ready and maintainable. The original author will be required to work with me, but he is also very proud of what he built (and rightfully so, as it performs a very important analysis). I want to make sure that whatever I do, I don't offend the original author in the process of this. – Kyle Feb 18 '20 at 02:55
  • 200KLOC, that's 500 days if programming at 400LOC/day. So 2 years of work. If you're alone. And if you clearly know what is to be done. Maybe it's worth after all to look at the root cause of the leaks? Or to refactor progressively, as Kain suggests... – Christophe Feb 18 '20 at 07:47
  • In [Things You Should Never Do, Part I](https://www.joelonsoftware.com/2000/04/06/things-you-should-never-do-part-i/), Joel Spolsky, one of the two founders of Stack Overflow, argues that rewriting code from scratch is usually the worst thing you can do. – pschill Feb 18 '20 at 08:16
  • @pschill: yep, but that article is about a software which was very successful in production before the rewrite happened. This question is about something which did not get so far (and has the huge risk of getting canceled before it ever will go live). – Doc Brown Feb 18 '20 at 09:00
  • As a rule of thumb, I would not recommend rewriting from scratch anything that takes more than two weeks to rewrite. – Lie Ryan Feb 18 '20 at 13:07
  • 1
    I agree with Joel's stance of never rewriting a big project and with the answers in that direction, so I won't repeat that in an answer. However I want to add the advice that you should get the "Working with legacy code" book from Michael Feathers. I'm not a book person and that's one of maybe 2 books I would actually advise other programmers to get. It's old, but it's almost like it gets better with age. Even if you don't work on legacy code it's incredibly helpful, and which programming language you use doesn't really matter either. – R. Schmitz Feb 18 '20 at 21:11

3 Answers3

8

Do you have complete, detailed, and comprehensive set of requirements, preferably as executable tests?

I'm going to guess: No as the writer probably hasn't discovered those yet...

In such a case, there is no guarantee that burning it down and rebuilding it will be even comparable. As you've already noted, you cannot even be sure that the code acts the way you would expect it to due to use of undefined behaviours. Not to mention you are even perplexed at to what the algorithm is itself due to the snarls of logic spread across the code base.

Instead, I would start by developing a battery of tests. Even if you simply grabbed some months worth of inputs/outputs and just check that the data matches.

At the very least this will give you some % chance of knowing if you have borked the code.

Refactor

Start by picking one function, one variable, or one class. See what you can do to improve it.

  • improve its name. Figure out what the function/variable/class does/represents.
  • pull a variable into a tighter scope.
  • find a couple of lines of sequential code, and pull it into its own named function.
  • wrap functions/variables into classes
  • split classes to increase cohesion, and introduce boundaries between concerns.
  • rephrase an undefined behaviour using well defined semantics.
  • etc... just small changes that improve your ability to read the code base and understand it.

Run the tests frequently to find anything obviously broken.

It would be best if you did this buddy programming with that scientist. This way you can improve the code base, train the scientist in how to write code better, and have the scientist input as to what each thing means/does.

It might also help if you institute Pull Requests with a mandatory code-review. This will allow you to push back on obviously bad code, so that things aren't getting worse.

Kain0_0
  • 15,888
  • 16
  • 37
  • I'm glad you mentioned tests, that was honestly my first thought too. On the plus side, the original author is still at the company, and management has tasked the two of us to get this codebase under control, and ready for production this Summer, as well as ensuring that we can maintain and run it for years to come. I've been told to do whatever necessary to accomplish this. I'm thinking there are 2 approaches, a) rewrite from scratch, or b) as you stated, refactor small parts over time. It seems to me though, that b) would take a tremendous amount of time. – Kyle Feb 18 '20 at 03:01
  • 1
    @Kyle My sympathises. As for complete rewrites, if it took `N` years to write a piece of software the first time, what are the chances that this is the universe in which the second attempt takes just six months? Burning it to the ground creates a vacuum, and vacuums take an enormous amount of work to maintain. The pressures that caused that software to be created in the first place still exist, and are still demanding change over time. This leaves a hole, or a game of catch up. Taking the refactor road is perhaps more gruelling but it still leaves you with working software after each change. – Kain0_0 Feb 18 '20 at 05:15
  • 1
    @Kyle: There might a third strategy between "rewrite from scratch" and "refactor small parts": set up a new empty project, and try to "migrate" the old code feature by feature into the new project. For each feature added, write tests and make it production ready. Whenever a feature is complete, release into production. That way, you can show real progress to your superiors, even if you don't get the full product running til summer. Of course, this means you see a sensible way of splitting the product into smaller, releasable parts, – Doc Brown Feb 18 '20 at 09:12
3

We are in a unique position now where the company is going to bet everything on this one piece of software written by someone who has never written production software before.

This is a huge risk.

That is why I would focus on the immediate problems (to prevent refactor-rage):

  • Is it an immediate problem that it doesn't run on multiple Operating Systems?
  • Is the program unreliable under all circumstances or can it reliably be used in a specific way and always produce good results when used that way?
  • How much time does it take to adjust the code for the most common changes? What is the minimal change that can be made to improve that?

In other words: don't fix what is not broken and focus on making a minimal viable product.

This focus would help to get the program up and running as soon as possible.

Then, maybe in parallel, start renaming the functions and variables in close collaboration with the original author. Mainly to learn and understand the domain and the solution. Do not change anything except for the names. The goal is understanding.

After the renaming, proper refactoring could be attempted.

Emond
  • 1,248
  • 8
  • 13
0

If you burn it down and start rom scratch I can guarantee you will end up with a mess that isn’t working. Your best chance is to hire an excellent professional with a bit of a masochistic streak and pay him to improve this code. Won’t be cheap.

gnasher729
  • 42,090
  • 4
  • 59
  • 119