Refactoring large code base legacy C++ project that mixed with C style

Question

Background:

Currently I am working on a C++ project. It's for business use, so this project is not open sourced. However, this is a project with a fairly large code base (around 10k lines of code) and a couple of external linked libraries and binaries. The problem is nearly 80% of the code base is written in C style rather than C++ style, but the main interface is written in C++11 standard and needs to be compiled in C++11. One of the most inconvenient parts of the C style code is plenty of goto statement are used, and I can tell the purpose of using goto is to ensure the code is thread safe and clean up the garbage right in time. (They worked) When I first learned C++, I started with C++11, and I think RAII is the modern fashion to replace the hacky style to ensure the code is thread safe and resources being used are cleaned up in time.

Puzzle:

I am assigned to add a series of new functions in the project. Some of my changes have to make in these C style code base. And if I changed one of them, why not refactor all of them in C++1x style? The conflict is given limited time, I can't finish refactoring all of them, not even 80%. And these C style code bases are also using some C style based libraries, I don't know if I changed my code from C style to C++ style, will these external libraries work as before? Will there be equivalent C++ libraries to replace those deprecated C libraries? So my puzzles are: Given limited time, how to refactor large code base legacy C++ project from C style to C++ style effectively? What is the trade off? Is the trade off worth considering?

Answer 1

C code using the goto cleanup style like this:

int function(int argument)
{
  int result;
  opaque_handle handle = NULL;
  char* text = NULL;
  size_t size;

  handle = extlib_create();
  if (!handle) {
    result = extlib_failed;
    goto cleanup;
  }
  size = extlib_size(handle, argument);
  text = malloc(size);
  if (!text) {
    result = oom;
    goto cleanup;
  }
  extlib_read(handle, text, size, argument);
  // etc.

  result = success;
  goto cleanup;

cleanup:
  if (handle) extlib_destroy(handle);
  if (text) free(text);
  return result;
}

can always refactored to C++ by wrapping each resource in a RAII wrapper and perhaps using some exception wrappers.

class extlib_handle {
  opaque_handle handle;
public:
  explicit extlib_handle(opaque_handle handle) : handle(handle) {}
  extlib_handle(const extlib_handle&) = delete;
  extlib_handle& operator =(const extlib_handle&) = delete;
  ~extlib_handle() { extlib_destroy(handle); }
  operator opaque_handle() const { return handle; }
};

template <typename V>
bool resize_nothrow(V& v, typename V::size_type sz)
try {
  v.resize(sz);
  return true;
} catch (std::bad_alloc&) {
  return false;
}

int function(int argument)
{
  extlib_handle handle(extlib_create());
  if (!handle) return extlib_failed;

  size_t size = extlib_size(handle, argument);
  std::vector<char> text;
  if (!resize_nothrow(text, size)) return oom;

  extlib_read(handle, text.data(), size, argument);
  // etc.

  return success;
}

This refactoring does not affect any code outside the function in any way, and allows you to slowly build up a collection of reusable RAII wrappers.

You can use this technique to refactor exactly the parts of the C code you actually touch, not any others. Touching any others introduces unnecessary risks (you could make a mistake and add a bug in code that would otherwise be untouched - this risk is mitigated in the code you have to touch anyway because there could already be new bugs there anyway, so it will receive more attention from testers, and because it makes writing new code less error-prone).

As usual, if you can, write tests. But chances are the code is not amenable to testing because the 3rd party dependencies can't be substituted. Refactoring for that is a time-consuming task.

Answer 2

I don't know if I changed my code from C style to C++ style, will these external libraries work as before? Will there be equivalent C++ libraries to replace those deprecated C libraries?

Without knowing which libraries you're using or which functions in them you are using, it's impossible to say. You can, of course, continue to call C functions from C++ and they should work as before if you're supplying the same inputs.

So my puzzles are: Given limited time, how to refactor large code base legacy C++ project from C style to C++ style effectively? What is the trade off? Is the trade off worth considering?

Given what you've said, I think the best course of action is to add your functions and if you can clean up the code that calls your functions or that your functions call into, then do that. I wouldn't touch anything that isn't directly connected to the additions you're making.

I don't recommend putting off all refactoring until management decides it needs to be done. In my experience, management will never decide it needs to be done, even if it will benefit you (as an organization) to do so. If you can do it a piece at a time while doing other stuff, that's usually a better approach. (Unless you know your management is willing to pay down technical debt in the future.)

There are a few tradeoffs here:

1. Refactoring everything now requires more time than you have, so it's not realistic
2. Refactoring only things you're already going to be touching may make those things take longer to complete, but you'll be left with a code base that's more up to date and easier to maintain
3. Waiting until this project is done and then convincing management to let you do some refactoring that won't have any outward appearance of new features is unlikely to be successful ever (in my experience).
4. Doing no refactoring and just adding your new functionality leaves you with exactly as much technical debt as before, and possibly more as you now have the original technical debt, and the interface with the newer stuff.

Answer 3

At college, my computer science professor used to say: "Our work is quick, cheap, and of high quality. You get to pick two of those."

In your case, you have limited time (so the work must be quick), and it only one developer is working on it (so not much money is spent on this work, therefore it is cheap). This means that the quality will inevitably suffer. And when quality suffers, that's when the cost goes up, more often than not at an exponential rate.

My advice to you: write as many automatic tests that verify the functionality of the code as you can before you refactor it. Then, after you refactor the code, you can confirm that the code still works the same way it did before refactoring.

The good thing about managers is that, in the end, they really only care about the money (which is really their job). So, you can always defend this position by pointing out that the costs of maintaining the old code and fixing the bugs in the new code that was written on top of the old code will outweigh the costs of refactoring.

Do not let the quality suffer. Anything else can be negotiable and susceptible to compromise.

Answer 4

Thank all the people who provide your advices to my question. Based on your suggestions and my own evaluation, I made this decision for this case.

What's wrong and not wrong with the goto?

I've read plenty of post for formatting C++ in some standard, one of the most common point is to avoid using goto, it's the assembly way of solving problem. If you have multiple gotos in your code in multiple scopes (if nested then worse), especially for large code base project, you will left a code that's less readable and less maintainable. However, if you do want to provide the flexibility to deal with your control flow, goto is acceptable in some case. Although modern languages used conditional statements to make branching more verbose, if someone used goto previously, might as well just keep them. In fact, some well-known well-behaved projects used goto as well, for ex: git.

Write C style code in C++:

Again C++ is a language with multiple paradigms. One of the purpose it is designed is to support different styles and make them compatible. So if existing C style code in a C++ project is working, we should left them as it is first.

Writing test before refactoring:

How do you know if your refactoring is working as your legacy code base did? Write automative test to ensure your refactored code produces the expected behavior. If you don't have these test, just don't do anything on them. "Let the grosses gross". Given limited time, I can't write a detailed test, what should I do? Write micro test and make micro change step by step. Only write test for the function you want to add in the legacy project.