You've just inherited a legacy C++ codebase, now what?

https://gaultier.github.io/blog/you_inherited_a_legacy_cpp_codebase_now_what.html

380 343 2 years ago

gaultier.github.io

keepamovin10• 2 years ago

It's funny. My first step would be

  0. You reach out to the previous maintainers, visit them, buy them tea/beer and chat (eventually) about the codebase. Learned Wizards will teach you much.

But I didn't see that anywhere. I think the rest of the suggestions (like get it running across platform, get tests passing) are useful stress tests likely to lead you to robustness and understanding however.

But I'd def be going for that sweeet, sweet low-hangin' fruit of just talking to the ol' folks who came that way before. Haha :)

Night_Thastus6• 2 years ago

IME, this only works if you can get regular help from them. A one-off won't help much at all.

keepamovin10• 2 years ago

Yeah you need to cultivate those relationships. But with a willing partner that first session will take you from 0 to 1 :)

Joel_Mckay4• 2 years ago

I've always found discussing why former employees left a project incredibly enlightening. They will usually explain the reality behind the PR given they are no longer involved in the politics. Most importantly they will often tell you your best case future with a firm.

Normally, employment agreements specifically restrict contact with former staff, or discussions of sensitive matters like compensation packages.

C++ is like any other language, in that it will often take 3 times longer to understand something than re-implement the same. If you are lucky, than everything is a minimal API lib, and you get repetitive examples of the use cases... but the cooperative OSS breadcrumb model almost never happens in commercial shops...

Legacy code bases can be hell to work with, as you end up with the responsibility for 14 years of IT kludges. Also, the opinions from entrenched lamers on what productivity means will be painful at first.

Usually, with C++ it can become its own project specific language variant (STL or Boost may help wrangle the chaos).

You have my sympathy, but no checklist can help with naive design inertia. Have a wonderful day. =)

mellutussa3• 2 years ago

A one-off won't help much at all.

Monumentally disagree. One-off session with a guy who knows the codebase inside out can save you days of research work. Plus telling you all about the problematic/historical areas.

raverbashing• 2 years ago

Easier said than done

My step 0 would be: run it through an UML tool to get a class diagram and other diagrams.

This will help you a lot.

Get the tests passing on your machine

Tests? On a C++ codebase? I like your optimism, rs

Joel_Mckay4• 2 years ago

No one has time to maintain the UML in production.

You may luck out with auto-generated documentation, but few use these tools properly (javadoc or doxygen). =)

GuB-422• 2 years ago

I wouldn't make it the first step. If you do, you will probably waste their time more than anything.

Try to work on it a little bit first, and once you get stuck in various places, now you can talk to the previous maintainers, it will be much more productive. They will also appreciate the effort.

guhidalg• 2 years ago

There’s a fine balance with no right or wrong answer. Previous maintainers will appreciate if you spent literally more than a second trying to understand before you reach out to them, but for your own sanity you should know when it’s time to stop and call for help.

keepamovin10• 2 years ago

Yeah, that's fair enough. I guess since we're already zero-indexed maybe my -1 step is Prep. Hahaha! :)

keepamovin10• 2 years ago

While this sounds good, it sort of assumes you're trying to solve some puzzle, and you're going to them for hints and answer. That's not the problem at hand tho: we're not asking them to solve some puzzle for us, we're just seeking understanding.

Because what we're actually doing is building capability to be able to work on a wide range of problems on our own. And getting guided by people who already know there way around the codebase and know what everything does is going to get you there quicker than any other method (unless docs are truly excellent, which in most cases they're not).

So, after reflecting on it more, I disagree. I think assuming you can poke around for yourself, and start to build up a picture of what things do, may end up giving you confident delusions, that could be then hard to unlearn, or at least blockers to further understanding. Better to go to the old guard with a blank slate mindset, and let them fill you in on the overview.

Then go away and work on it, having that guidance in store, and then (if you were friendly enough in the first meet) come back for additional. That is what I think people would truly appreciate.

Think about it: assume you were someone who generally had time to help people coming to you for assistance, would you not love to sit someone down and give them an in depth download of the big picture, and details overview, of this thing into which you poured great love and devotion (or at least, many hours, sweat and dollars?). I think you would.

Anyway, it's a reminder to me that what often sounds right in the moment, and is easy to agree with, actually turns out to not be the best idea. I'm always eager to see another side, and empathize with someone else's view, but the lesson for me is: I often take that too far, and in doing so, forget what I originally came in with, forget my own frame, and forget what's true, so eager am I to see it from someone else's point of view. So that's a big lesson for me. Thanks for being part of it! Haha! :)

theamk2• 2 years ago

Maybe do a quick look at codebase first so you can identify biggest WTF's and ask about them.

After all, if you have inherited a codebase with no tests, with build process which fails every other time, with unknown dependency info, and which can only be built on a single server with severely outdated OS... are you sure the previous maintainer is a real wizard and all the problems are result of not enough time? Or are they a "wizard" who keep things broken for job security and/or because they don't want to learn new things?

keepamovin10• 2 years ago

Yes! Good idea. Locking in at -1. Hahah! :)

vaylian• 2 years ago

Even if that person is not a great archwizard, they still have more experience in that project than you and you will probably learn some things that will make your life less miserable, because you will better understand what to expect and what kind of failed solutions have been tried before.

lelanthran2• 2 years ago

It's funny. My first step would be
0. You reach out to the previous maintainers, visit them, buy them tea/beer and chat (eventually) about the codebase. Learned Wizards will teach you much.

Have you ever tried that? This is legacy code. Even if the handover was yesterday, they cannot tell you about anything useful they did more than 6m in the past.

And that's the best-case scenario. The common-case is "That's the way I got it when it was handed over to me" answer to every question you ask.

mst6• 2 years ago

I have, and assuming your predecessor doesn't mind, you still get enough useful answers to make it worth it a lot of the time.

Especially if you manage to get to just chatting about the project - at some point they'll almost certainly go "oh! while I remember," followed by the answer to a question you didn't realise you should've asked.

The value is often in the commiseration rather than the interrogation, basically.

fransje265• 2 years ago

    > You reach out to the previous maintainers, visit them

I could have brought them flowers, and shared a moment of silence contemplating eternity. I don't know if it would significantly have helped understanding the code base though..

planede4• 2 years ago

You mean the guys the company laid off last week?

keepamovin10• 2 years ago

Ouch, yeah. I appreciate your topicality with this comment given the recent saga of tech layoffs. Much luck to you if you're in that experience right now!! Say you got fired form a S.Eng job at 350K yearly and some young grunt on 165K slid into your DMs and wanted to meet to chat. What would make you agree to that? I think it's unlikely, but perhaps if they reflected to you all the things you already thought were stupid about that company, and that you suffered with. You would want to help them as in their suffering, you could see your own, despite getting laid off. I think in that situation, there's a chance you might actually go over the codebase with them, in a conversation that mixed 1 part that, with 1 part self-commiserating and complaining/airing/venting about the org.

What at first seems an unlikely pairing, I think, given the right "relational config" in moment could actually end up working pretty well. Seems there's plenty opportunities for such a coupling to have many fruitful outputs for both parties.

TrackerFF2• 2 years ago

I was once tasked with deploying a piece of software on a closed network (military), to run on a old custom OS - it wasn't a huge program, around 50k lines of code.

I did encounter a bunch of bugs and problems underway, and wanted to reach out to the devs that wrote it - as it was customer made for my employer.

Welp, turns out it was written by one guy/contractor, and that he had passed away a couple of years earlier.

At least in the defense industry you'll find these sort of things all the time. Lots of custom/one-off stuff, made for very specific systems. Especially on the hardware side it is not uncommon that the engineers that made the equipment are either long gone or retired.

keepamovin10• 2 years ago

Welp, you might have needed to consult a seance for that one.

Interesting point about the "author expiry" window. I was thinking about that today regarding something else:

Let's say in 20 years time, no database code has been updated for the last 20 years. And all the people who worked on it, can't remember anything about it. Yet, it still works.

That means that everyone who uses that database everyday, doesn't know how it works. They believe that it works, this belief is widespread. And the database providing results to queries, is a real thing. And it does work -- but nobody knows how.

This is common. I don't know in any detail how the MacBook I use works. But it does. I don't know how many things I use actually work. But they do work.

It seems the only difference, in the world of things that "work", and which most people who use them do not understand how they work, is that there are two classes of things: those things for which there is a widespread belief that they do work; and those things for which the belief that they work, is not widespread. But in either case, they work.

rwmj• 2 years ago

Surprisingly good advice. In a similar vein, Joel's 12 steps to better software: https://www.joelonsoftware.com/2000/08/09/the-joel-test-12-s...

leecarraher• 2 years ago

create bindings and externalize function libraries for other languages, hope to your prefered deity nothing breaks

Night_Thastus6• 2 years ago

This adds additional problems. IE, Start replacing legacy C++ with Python, now debugging and following the flow of the code becomes very difficult.

bluGill16• 2 years ago

If it is C++ I wouldn't think about python in most cases. Rust should come to mind. Ada, or D are other options you sometimes hear about.

sjc02060• 2 years ago

A good read. We recently did "Rewrite in a memory safe language?" successfully. It was something that shouldn't have been written in C++ in the first place (it was never performance sensitive).

tehnub3• 2 years ago

Would you mind sharing what language you used?

jstimpfle• 2 years ago

Probably not a project spanning more than 3 decades of development and millions of lines of code?

throwaway20375• 2 years ago

Do you have a public write-up (blog post)? If yes, you should post it on HN. It would probably generate lots of interesting conversation.

Jtsummers6• 2 years ago

I'd swap 2 and 3. Getting CI, linting, auto-formatting, etc. going is a higher priority than tearing things out. Why? Because you don't know what to tear out yet or even the consequence of tearing them out. Linting (and other static analysis tools) also give you a lot of insight into where the program needs work.

Things that get flagged by a static analysis tool (today) will often be areas where you can tear out entire functions and maybe even classes and files because they'll be a re-creation of STL concepts. Like homegrown iterator libraries (with subtle problems) that can be replaced with the STL algorithms library, or homegrown smart pointers that can just be replaced with actual smart pointers, or replacing the C string functions with C++'s on string class (and related classes) and functions/methods.

But you won't see that as easily until you start scanning the code. And you won't be able to evaluate the consequences until you push towards more rapid test builds (at least) if not deployment.

broken_broken_2• 2 years ago

Fair point!

dralley• 2 years ago

On the flip side, auto-formatting will trash your version history and impede analysis of "when and why was this line added".

skrebbel• 2 years ago

You can ignore commits from git blame by adding them to a .gitattributes file.

This is assuming Git of course, which is not a given at all for the average legacy c++ codebase.

Jtsummers6• 2 years ago

I'm not hardcore on auto-formatters, but I think their impact on code history is negligible in the case of every legacy system I've worked on. The code history just isn't there. These aren't projects that used git until recently (if at all). Before that they used something else, but when they transitioned they didn't preserve the history. And that's if they used any version control system. I've tried to help teams whose idea of version control was emailing someone (they termed them "QA/CM") to make a read-only backup of the source directory every few months (usually at a critical review period in the project, so a lot of code was changed between these snapshots).

That said, sure, skip them if you're worried about the history getting messed up or use them more selectively.

PreachSoup• 2 years ago

On per file level it's just 1 commit. It's not really a big deal

exDM69• 2 years ago

clang-format can be applied to new changes only, for this very reason.

Adding it will remove white space nitpicking from code review, even if it isn't perfect.

IshKebab2• 2 years ago

I believe you can configure `git blame` to skip a specific commit. But in my experience it doesn't matter anyway for two reasons:

1. You're going to reformat it eventually anyway. You're just delaying things. The best time to plant a tree, etc.

2. If it's an old codebase and you're trying to understand some bit of code you're almost always going to have to walk through about 5 commits to get to the original one anyway. One extra formatting commit doesn't really make any difference.

lpapez• 2 years ago

You can instruct git to ignore specific commits for blame and diff commands.

See "git blame ignore revs file".

Intended use is exactly to ignore bulk changes like auto formatting.

duped• 2 years ago

This is another reason why you should track important information in comments alongside the code instead of trusting VCS to preserve it in logs/commit messages, and to reject weird code missing comments from being merged.

Not saying that fixes decades of cruft because you shouldn't change files without good reason and non-white space formatting is not a good reason, but I'm mentioning it because I've seen people naively belief bullshit like "code is self explanatory" and "the reason is in the commit message"

Just comment your code folks, this becomes less of a problem

mb7733• 2 years ago

How does reformatting trash the history? It's one extra commit..

I guess if it splits or combines lines that could cause some noise if you really want the history of a single line... But that happens all the time, and I don't see how it would really prevent understanding the history. You can always do a blame on a range of lines.

Maybe I'm missing something though, genuinely curious for a concrete example where reformatting makes it hard to understand history!

politician• 2 years ago

Nit: The post scopes "tearing things out" to dead code as guided by compiler warnings and unsupported architectures.

If going the route, I'd recommend commenting out the lines rather than removing them outright to simplify the diffs at least until you're ready to squash and merge the branch.

SAI_Peregrinus• 2 years ago

Better to use `#if` or `#ifdef` to prevent compilation. C & C++ don't support nested comments, so you can end up with existing comments in the code ending the comment block.

btown• 2 years ago

CI is different from the others, here! At minimum, building a "happy path(s)" test harness that can run with replicable results, and will run on every one of your commits, is a first step, and also helps to understand the codebase.

And you're jumping around - and you'll have to! - odds are you'll have a bunch of things changed locally, and might accidentally create a commit that doesn't separate out one concern from another. CI will be a godsend at that point.

jasonwatkinspdx• 2 years ago

Yeah, I've done a fair bit of agency work dropping in to rescue code bases, and the first thing I do is run unit tests and check coverage. I add basic smoke tests anywhere they're missing. This actually speeds me up, rather than slowing me down, because once I have reasonably good coverage I can move dramatically faster when refactoring. It's a small investment that pays off.

thrwyexecbrain• 2 years ago

I would absolutely not recommend auto-formatting a legacy codebase. In my experience large C++ projects tend to have not only code generation scripts (python/perl/whatever) but also scripts that parse the code (usually to gather data for code generation). Auto formatting might break that. I have even seen some really cursed projects where the _users_ parsed public header files with rather fragile scripts.

Jtsummers6• 2 years ago

I was listing the items in the original article's #3 and saying I'd move them up to #2 before I'd go about excising portions of the project, the original #2. I still stand by that. But you can read my other comment where I don't really defend auto-formatting to see that I don't care either way. I made it about four hours ago so maybe you missed it if you didn't refresh the page in the last few hours.

ecshafer• 2 years ago

This is pretty great advice for any legacy code project. Even outside of C++ there is a huge amount of code bases out there that do not compile/run on a dev machine without tons of work. I once worked on a Java project that due to some weird dependencies, the dev mode was to run a junit test which started spring and went into an infinite loop. Getting a standard run to work helped a ton.

bluGill16• 2 years ago

The difference between greenfield and legacy code is just a few years. So learn to work with legacy code and how to make it better over time.

hesdeadjim• 2 years ago

Start grinding leetcode and find another gig?

bun_terminator6• 2 years ago

Rewrite in a memory safe language?

like c++11 and later?

z_open4• 2 years ago

How is that memory safe? Even vector out of bounds index is not memory safe.

bun_terminator6• 2 years ago

You can access a vector with a function that throws an exception if you so desire

evouga• 2 years ago

It's funny; I spent a couple of hours last week helping some students debug out-of-bounds indices in their Rust code.

I've written bugs that would have been caught by the compiler in a memory-safe language. I think the last time was maybe in 2012 or 2013? I still write plenty of bugs today but they're almost all logic errors that nothing (short of AI tools) could have caught.

bluGill16• 2 years ago

vector.at() is memory safe. You get a choice. Easy to ban [] if you cannot statically prove it is safe.

C++11 isn't the most memory safe language, but C++11 is a lot safer than older versions, and C++23 is better yet. I'm expecting true memory safety in C++26 (time will tell), but it will be opt-in profiles which isn't ideal.

devnullbrain• 2 years ago

Oops, shared_ptr circular reference.

Oops, null smart pointer.

Oops, UB made my reference null.

Oops, invalidated iterator.

Oops, aliased pointers.

Oops, race condition.

Oops, recursion.

Oops, moved-from object is partially formed.

girafffe_i3• 2 years ago

I heard about Rust recently.

sealeck2• 2 years ago

rm -r

Problem solved

dtx1• 2 years ago

C++
Not even once

GuB-422• 2 years ago

If you mean "rewrite from scratch", believe me, it is the worst thing you can do. I speak from experience, it is tempting but the few times I have done that, a few months later as I get burnt, I could only think of how an idiot who never learn I was.

Legacy code is like that because it went through many bugfixes and addressing weird requirements. Start over and you lose all that history, and it is bound to repeat itself. That weird feature that makes no sense, as it turns out, makes a lot of sense for some users, and that why it has been implemented in the first place. And customers don't care about your new architecture and fancy languages, they want their feature back, otherwise they won't pay.

Another way to look at it is when you asked to maintain a legacy code base, that's because that's software that has been in use for a long time. If it was that bad, it would have been dropped long ago, or maybe even cancelled before it got any use. Respect software that is used in production, many don't reach that stage.

Of course there are exceptions to that rule, but the general idea about rewriting from scratch is: "no" means "no", "maybe" means "no", and "yes" means "maybe".

sealeck2• 2 years ago

I'm being like 1000% facetious, and agree that rewrites are bad.

bluGill16• 2 years ago

I have been involved in a successful rewrite. It cost billions of dollars and many years when the code wasn't working so the old system was still in use. We also ended up bringing over some old code directly just to get something - anything - functional at all. For many years my boss kept the old version running on his desk because when there was a question that old system was the requirements.

Today we only have to maintain the new system (the old is no longer sold/supported), and the code is a lot better than the old one. However I suspect we could have refactored the old system in place for less time/money and been shipping all the time. Now we have a new system and it works great - but we already have had to do significant refactors because some new requirement came along that didn't fit our nice architecture.

Night_Thastus6• 2 years ago

worry not, by adding std::cmake to the standard library and you’ll see how it’s absolutely a game changer

I'm pretty sure my stomach did somersaults on that.

But as for the advice:

Get out the chainsaw and rip out everything that’s not absolutely required to provide the features your company/open source project is advertising and selling

I hear you, but this is incredibly dangerous. Might as well take that chainsaw to yourself if you want to try this.

It's dangerous for multiple reasons. Mainly it's a case of Chesterton's fence. Unless you fully understand why X was in the software and fully understand how the current version of the software is used, you cannot remove it. A worst case scenario would be that maybe a month or so later you make a release and the users find out an important feature is subtly broken. You'll spend days trying to track down exactly how it broke.

Make the project enter the 21st century by adding CI, linters, fuzzing, auto-formatting, etc

It's a nice idea, but it's hard to do. One person is using VIM, another is using emacs, another is using QTCreator, another primarily edits in VSCode.. Trying to get everyone on the same page about all this is very, very hard.

If it's an optional step that requires that they install something new (like commit hook) it's just not going to happen.

Linters also won't do you any good when you open the project and 2000+ warnings appear.

aaronbrethorst• 2 years ago

An optional step locally like pre-commit hooks should be backed up by a required step in the CI. In other words: the ability to run tests locally, lint, fuzz, format, verify Yaml format, check for missing EOF new lines, etc, should exist to help a developer prevent a CI failure before they push.

As far as linters causing thousands of warnings to appear on opening the project, the developer adding the linter should make sure that the linter returns no warnings before they merge that change. This can be accomplished by disabling the linter for some warnings, some files, making some fixes, or some combination of the above.

zer00eyz3• 2 years ago

> It's a nice idea, but it's hard to do. One person is using VIM, another is using emacs, another is using QTCreator, another primarily edits in VSCode.. Trying to get everyone on the same page about all this is very, very hard.

This is what's wrong with our industry, and it's no longer an acceptable answer. We're supposed to be fucking professional, and if a job needs to build a tool chain from the IDE up we need to learn to use it and live with it.

Built on my machine, with my IDE, the way I like it and it works is not software. It's arts and fucking crafts.

cratermoon7• 2 years ago

If you're saying everyone should agree on the same IDE and personal development toolset, I disagree, sort of.

The GP was suggesting the effort to add CI, linters, fuzzing, auto-formatting, etc was too hard. If that can be abandoned entirely, perhaps the legacy codebase isn't providing enough value, and the effort to maintain it would be better spent replacing it. But the implication is that the value outweighs the costs.

Put all the linters, fuzzing, and format checking in an automated build toolchain. Allow individuals to work how they want, except they can't break the build. Usually this will reign in the edge cases using inadequate tools. The "built on my machine, with my IDE, the way I like it and it works" is no longer the arbiter of correct, but neither does the organization have to deal with the endless yak shaving over brace style and tool choice.

otabdeveloper4• 2 years ago

every single carpenter in the world should use the exact same make and model of saw, for, uh, professionalism reasons

saagarjha4• 2 years ago

Software is arts and crafts :)

electroly• 2 years ago

It's a nice idea, but it's hard to do. One person is using VIM...

The things the author listed there are commonly not IDE integrated. I've never seen a C++ development environment where cpplint/clang-tidy and fuzzers are IDE integrated, they're too slow to run automatically on keystrokes. Auto-formatting is the only one that is sometimes integrated. All of this stuff you can do from the command line without caring about each user's chosen development environment. You should definitely at least try rather than giving up before you start just because you have two different text editors in use. This is C++; if your team won't install any tools, you're gonna have a bad time. Consider containerizing the tools so it's easier.

gpderetta• 2 years ago

Clangd will happily run clang-tidy as part of completion/compile-as-you-type/refactor/auto independent.

On the editor/IDE of your choose.

I wouldn't call it fast, but it is quite usable.

throwaway20375• 2 years ago

I've never seen a C++ development environment where cpplint/clang-tidy and fuzzers are IDE integrated

CLion from JetBrains has clang-tidy integrated (real-time).

chlorion• 2 years ago

In Emacs I have clangd and clang-tidy running on each key stroke!

The project size is probably a lot smaller than what most people are working on though, and I have a fast CPU and NVME disk, but it's definitely possible to do!

I'm not sure about the fuzzer part though.

IshKebab2• 2 years ago

One person is using VIM, ...

I don't get your point. You know you can autoformat outside editors right? Just configure pre-commit and run it in CI. It's trivial.

If it's an optional step that requires that they install something new (like commit hook) it's just not going to happen.

It will because if they don't then their PRs will fail CI.

This is really basic stuff, but knowledge of how to do CI and infra right does seem to vary massively.

selfmodruntime• 2 years ago

It's a nice idea, but it's hard to do. One person is using VIM, another is using emacs, another is using QTCreator, another primarily edits in VSCode.. Trying to get everyone on the same page about all this is very, very hard.

I must have missed the memo where I could just say no to basic things my boss requires of me. You know, the guy that pays my salary.

saagarjha4• 2 years ago

As others have mentioned, none of these things actually change your development workflow. But if they did, you do have the ability to say no. If your boss fails to understand that you have an environment that you're productive in, that sounds like a bad place to work.

rcxdude• 2 years ago

You can, in fact, just do that. You are allowed. You may get fired if you are too much of a headache, but employers are actually flexible.

Kamq• 2 years ago

It's a nice idea, but it's hard to do. One person is using VIM, another is using emacs, another is using QTCreator, another primarily edits in VSCode.. Trying to get everyone on the same page about all this is very, very hard.

Bullshit, all of these (and additionally C lion) are fairly easy to configure these for, with the possible exception of QTCreator (not a ton of experience on my end).

Just make it a CI requirement, and let everyone figure it out for their own tools. If you can't figure that out, you get to run it as a shell script before you do your PRs. If you can't figure that out, you probably shouldn't be on a legacy C++ project.

theamk2• 2 years ago

It's dangerous for multiple reasons. Mainly it's a case of Chesterton's fence. Unless you fully understand why X was in the software...

If this is a function that no one links to, and your project does not mess with manual dynamic linking (or the function is not exposed), then it's pretty safe to remove it. If it's internal utility which does not get packaged into final release package, it is likely be safe to remove too. If it's a program which does not compile because it requires Solaris STREAMS and your targets are Linux + MacOS - kill it with fire.

(Of course removing function calls, or removing functionality that in-use code depends on, is dangerous. But there is plenty of stuff which has no connection to main code)

bluetomcat• 2 years ago

Been there, done that. Don't be a code beauty queen. Make it compile and make it run on your machine. Study the basic control-flow graph starting from the entry point and see the relations between source files. Debug it with step-into and see how deep you go. Only then can you gradually start seeing the big picture and any potential improvements.

johngossman2• 2 years ago

Absolutely. Read the code. Step through with a debugger. Fix obvious bugs. If it’s legacy and somebody is still paying to have it worked on, it must mostly work. Changing things for “cleanliness and modernization” is likely to break it.

cratermoon7• 2 years ago

Fix obvious bugs.

Be careful about that. Hyrum's Law and all.

bluGill16• 2 years ago

In my experience it takes at least a year straight working with code before you can form an opinion on if it is beautiful or not. People who have not worked in a code base for that long do not understand what is a beautiful design corrupted by the real world vs what is ugly code. Most code started out with a beautiful design but the real world forced ugly on it - you might be able to improve this a little with full rewrite but the real world will still force a lot of ugly on you. However some code really is bad.

sk110013• 2 years ago

Is it worth getting more into C++ in 2024? Lots of interesting jobs in finance require it but it seems almost impossible to get hired without prior experience (with C++ and in finance).

optimalsolver• 2 years ago

Yes.

I switched from Python to C++ because Cython, Numba, etc. just weren't cutting it for my CPU-intensive research needs (program synthesis), and I've never looked back.

sk110013• 2 years ago

My question isn't whether it's a good fit for a specific project, I'm more interested in whether it's a good career choice e.g. can you get a job using C++ without C++ experience; how realistic is it to ramp up on it quickly; whether you're likely to end up with some gnarly legacy codebase as described in the OP; is it worth pursuing this direction at all.

d_sem• 2 years ago

Depends on the industry you are interested in entering.

My myopic view of the world has seen the general trend from C to C++ for realtime embedded applications. For example: in the Automotive Industry all the interesting automotive features are written in C++.

BobbyTables2• 2 years ago

Great, now they can all inherit from “HopeIDontCrashToday”

davidw• 2 years ago

Well, tomorrow is the "who's hiring?" thread...

tehnub3• 2 years ago

I enjoyed the article and learned something. But I've been wondering: When people say "rewrite in a memory-safe language", what languages are they suggesting? Is this author rewriting parts in Go, Java, C#? Or is it just a smirky, plausibly deniable way of saying to rewrite it in Rust?

broken_broken_2• 2 years ago

Author here, thanks! A second article will cover this, but the bottom line is that it entirely depends on the team and the constraints e.g. is a GC an option (then Go is a good option), is security the highest priority, etc.

I’d say that most C++ developers will generally have an easy time using Rust and will get equivalent performance.

But sometimes the project did not have a good reason to be in C++ in the first place and I’ve seen successful rewrites in Java for example.

Apple is rewriting some C++ code in Swift, etc. So, the language the team/company is comfortable with is a good rule of thumb.

tehnub3• 2 years ago

Makes sense, thanks!

avgcorrection3• 2 years ago

So you saw a post about C++, it didn’t mention “Rust” once, mentioned “memory safe” languages which there are dozens of, and yet found a way to shoehorn in a dismissive comment about a meme. Nice.

We’ve reached the rewrite-in-rust meme stage of questioning whether the author is a nefarious crypto-Rust programmer in lieu of not being able to complain about it (since it wasn’t brought up!).

bsdpufferfish• 2 years ago

(author actually shows up to advocate for rust)

huqedato• 2 years ago

Whenever I inherited a project containing legacy code, regardless of the frameworks, tools, or languages used, we always found it necessary to drop it and begin anew. Despite my efforts to reuse, update, or refactor it, we inevitably reached a point where it was unusable for further development.

Kapura• 2 years ago

Get out the chainsaw and rip out everything that’s not absolutely required to provide the features your company/open source project is advertising and selling

Great advice! People do not often think about the value of de-cluttering the codebase, especially _before_ a refactor.

cratermoon7• 2 years ago

Kill It with Fire https://nostarch.com/kill-it-fire

myrmidon• 2 years ago

Really liked it! Especially the "get buy in" is really good advice-- always stressing how the effort spent on refactoring actually improves things, and WHY its necessary.

Something that's kinda implied that I would really stress: Establish a "single source of truth" for any release/binary that reaches production/customers, before even touching ANY code (Ideally CI. And ideally builds are reproducible).

If you build from different machines/environments/toolchains, its only a matter of time before that in itself breaks something, and those kinds of problems can be really "interesting" to find (an obscure race condition that only occurs when using a newer compiler, etc.)

bArray• 2 years ago

3. Make the project enter the 21st century by adding CI, linters, fuzzing, auto-formatting, etc

I would break this down:

a) CI - Ensure not just you can build this, but it can be built elsewhere too. This should prevent compile-based regressions.

b) Compiler warnings and static analysers - They are likely both smarter than you. When it says "warning, you're doing weird things with a pointer and it scares me", it's a good indication you should go check it out.

c) Unit testing - Set up a series of tests for important parts of the code to ensure it performs precisely the task you expect it to, all the way down to the low level. There's a really good chance it doesn't, and you need to understand why. Fixing something could cause something else to blow up as it was written around this bugged code. You also end up with a series of regression tests for the most important code.

n) Auto-formatting - Not a priority. You should adopt the same style as the original maintainer.

5. If you can, contemplate rewrite some parts in a memory safe language

The last step of an inherited C++ codebase is to rewrite it in a memory safe language? A few reasons why this probably won't work:

1. Getting resources to do additional work on something that isn't broken can be difficult.

2. Rather than just needing knowledge in C++, you now also need knowledge in an additional language too.

3. Your testing potentially becomes more complex.

4. Your project likely won't lend itself to being written in multiple languages, due to memory/performance constraints. It must be a significantly hard problem that you didn't just write it yourself.

5. You have chosen to inherit a legacy codebase rather than write something from scratch. It's an admittance that you don't have some resource (time/money/knowledge/etc) to do so.

jpc02• 2 years ago

The last step of an inherited C++ codebase is to rewrite it in a memory safe language

Simply getting rid of any actually memory unsafe C++ and enforcing guidelines will do this for you in the C++ codebase.

"Rewrite it in X" only adds complexity because it's the flavour of the month as you said in your comment.

Author is already doing the work of rewriting large chunks of the codebase in C++, they may as well follow and implement a more restrictive subset of the language, I find High integrity C++ to be good. If I can get my hands on the latest MISRA standard that is likely good as well. These may not be "required" but they specify what is enforced in <enter "safe" language here>. So instead of having to reskill your entire devteam on a new language which has many many sharp edges, how about just having your dev team use the language they already know and enforce guidelines to avoid known footguns.

dieortin4• 2 years ago

How would you get rid of any memory unsafe C++? Isn’t that just another way of saying “do not make mistakes”?

grandinj• 2 years ago

This is generally the same path that LibreOffice followed. Works reasonably well.

We built our own find-dead-code tool, because the extant ones were imprecise, and boy oh boy did they find lots of dead stuff. And more dead stuff. And more dead stuff. Like peeling an onion, it went on for quite a while. But totally worth it in the end, made various improvements much easier.

Scubabear68• 2 years ago

The “rip everything out” step is not recommended. You will break things you don’t understand, invoke Chesterson’s Fence, and create enormous amounts of unnecessary work for yourself.

Make it compile, automate what you can, try not to poke the bear as much as you can, pray you can start strangling it by porting pieces to something else over time.

beanjuiceII• 2 years ago

the whitehouse says i should RiiR

jeffrallen• 2 years ago

This was my job at Cisco. But it was a C code base, which used nonstandard compiler extensions, and so could not be built without the legacy compilers with their locally made extensions. Also the "unit tests" were actually hardware-in-the-loop tests. And the Makefiles referenced NFS filesystems automounted from global replicas, but none of them were on my continent.

Fun times. Don't work there anymore. Life is good. :)

mk_chan2• 2 years ago

I’m not sure why there’s so much focus on refactoring or improving it. When a feature needs to be added that can just be tacked onto the code, do it without touching anything else.

If it’s a big enough change, export whatever you need out of the legacy code (by calling an external function/introducing a network layer/pulling the exact same code out into a library/other assorted ways of separating code) and do the rest in a fresh environment.

I wouldn’t try to do any major refactor unless several people are going to work on the code in the future and the code needs to have certain assumptions and standards so it is easy for the group to work together on it.

dj_mc_merlin• 2 years ago

The post argues against major refactors. The incremental suggestions it gives progressively make the code easier to work with. What you suggest works until it doesn't -- something suddenly breaks when you make a change and there's so much disorganized stuff that you can't pinpoint the cause for much longer than necessary. The OP is basically arguing for decluttering in order to be able to do changes easier, while still maintaining cohesion and avoiding a major rewrite.

bluGill16• 2 years ago

The right answer depends on the future. I've worked on C++ code where the replacement was already in the market but we had to do a couple more releases of the old code. Sometimes it is adding the same feature to both versions. There is a big difference in how you treat code that you know the last release is coming soon and code where you expect to maintain and add features for a few more decades.

mk_chan2• 2 years ago

Yes, you have to expect the future (or even better if your manager/boss already has expectations you can adopt to begin with) and then choose the right way to tackle the changes required. That's why I laid out 3 possible cases the last of which points out that I prefer to refactor primarily when there's a lot of work incoming on the codebase. Personally, I don't see much value in refactoring code significantly if you alone are going to be editing it because refactoring for ease of editing + the cost of editing in the refactored codebase is often less than just eating the higher cost of editing in the pre-refactored codebase and you don't reap the scaling benefits of refactoring as much. However, like I mentioned in the above paragraph, it depends. In the end it's all about managing the debt to get the most out of it in a _relatively_ fixed time period.

FpUser2• 2 years ago

"When a feature needs to be added that can just be tacked onto the code, do it without touching anything else."

In few lucky cases. In real life new feature is most likely change in behavior of already existing one and suddenly you have to do some heavy refactoring in numerous places.

convolvatron2• 2 years ago

if you're going to own it for the foreseeable future. then own it. learn it, refactor it, test the hell of out of it. otherwise you're never going to be able to debug or extend it.

one thing I always do is throwaway major refactors. its the fastest way for me to learn what the structure is, what depends on what, and what's really kinky. and I might just learn enough to do it for real should it become necessary.

VyseofArcadia• 2 years ago

Get out the chainsaw and rip out everything that’s not absolutely required to provide the features your company/open source project is advertising and selling

Except every legacy C++ codebase I've worked on is decades old. Just enumerating the different "features" is a fool's errand. Because of reshuffling and process changes, even marketing doesn't have a complete list of our "features". And even it there was a complete list of features, we have too many customers that rely on spacebar heating[0] to just remove code that we think doesn't map to a feature.

That's if we can even tease apart which bits of code map to a feature. It's not like we only added brand new code for each feature. We also relied on and modified existing code. The only code that's "safe" to remove is dead code, and sometimes that's not as dead as you might think.

Even if we had a list of features and even if code mapped cleanly to features, the idea of removing all code not related to "features your company is advertising or selling" is absurd. Sometimes a feature is so widely used that you don't advertise it anymore. It's just there. Should Microsoft remove boldface text from Word because they're not actively advertising it?

The only way this makes sense is if the author and I have wildly different ideas about what "legacy" means.

[0] https://xkcd.com/1172/

Palomides• 2 years ago

hard agree

removing a feature is possibly the most politically intractable thing you can try to do with a legacy codebase, almost never worth trying

lelanthran2• 2 years ago

> Get out the chainsaw and rip out everything that’s not absolutely required to provide the features your company/open source project is advertising and selling
Except every legacy C++ codebase I've worked on is decades old. Just enumerating the different "features" is a fool's errand. Because of reshuffling and process changes, even marketing doesn't have a complete list of our "features".

Yeah, this struck me also, and your post should be modded up more. Anyone with significant experience in development knows what "Legacy" means.

Regardless of the language, after a specific point in a product's lifetime you cannot "know" all the features. Just not possible, no matter how well you think you documented it.

In an old product, every single line of code is there because of a reason that is not in the docs. Some examples that I've seen:

1. Using `int8_t` because at some point we integrated a precompiled library that was compiled with signed char, and we want warnings to pop up when we mix signs.

2. Wrote our own stripped-down SSL library because OpenSSL was not EMV certified at the time and did not come with the devkit. Now callers depend on a feature in our own library.

3. Client calls our DLL with Windows-specific UTF-16 strings. That's why that function has three variants that take 3 different types of strings.

4. This library can't be compiled with any gcc/glibc newer than X.Y, because the compiled library is loaded with `dlopen` in some environments.

5. We have our own 'safe' versions of string functions, because MSVC which takes the same parameter types for those functions assigns different meanings to the `size` parameter.

6. Converting fixed-precision floats to an int, performing the additions, and then converting the last division is faster and more accurate, but the test suite at the client expects the cumulative floating point errors and the test will fail.

Not to mention uncountable "marketing said this is not offered as a feature, but client depends on it" things.

professorTuring• 2 years ago

How good is AI refactoring the code? Haven’t tried it yet, but… as someone who has need to work on tons of legacy in the past… looks interesting!

bluGill16• 2 years ago

Very mixed. Sometimes great, but you have too watch it close as once in a while it will do garbage.

There is a lot of non-AI refactoring for C++ these days that is very good. And many more tools that will point to areas that there is a problem and often a manual fix of those areas is "easy".

eschneider9• 2 years ago

Some good advice here, and some more...controversial advice here.

After inheriting quite a few giant C++ projects over the years, there are a few obvious big wins to start with:

* Reproducible builds. The sanity you save will be your own. Pro-tip: wrap your build environment with docker (or your favorite packager) so that your tooling and dependencies become both explicit and reproducable. The sanity you save will be your own.

* Get the code to build clean with -Wall. This is for a couple of reasons. a) You'll turn up some amount of bad code/undefined behavior/bugs this way. Fix them and make the warning go away. It's ok to #pragma away some warnings once you've determined you understand what's happening and it's "ok" in your situation. But that should be rare. b) Once the build is clean, you'll get obvious warnings when YOU do something sketchy and you can fix that shit immediately. Again, the sanity you save will be your own.

* Do some early testing with something like valgrind and investigate any read/write errors it turns up. This is an easy win from a bugfix/stability point of view.

* At least initially, keep refactorings localized. If you work on a section and learn what it's doing, it's fine to clean it up and make it better, but rearchitecting the world before you have a good grasp on what's going on globally is just asking for pain and agony.

SleepyMyroslav• 2 years ago

I would not call that 'controversial'. In the internet days people call this behavior trolling for a reason. The punchline about rewriting code in different language gives an easy hint at where this all going.

PS. I have been in the shoes of inheriting old projects before. And I hope i left them in better state than they were before.

dataflow4• 2 years ago

Step 0: reproducible builds (like you said)

Step 1: run all tests, mark all the flaky ones.

Step 2: run all tests under sanitizers, mark all the ones that fail.

Step 3: fix all the sanitizer failures.

Step 4: (the other stuff you wrote)

eschneider9• 2 years ago

Just a note on legacy tests: Step 0.5: understand the tests. They need to be examined to see if they've rotted or not. Tests passing/failing doesn't really mean code under test works or not. The tests might have been abandoned under previous management and don't accurately reflect how the code is _supposed_ to be working.

daemin• 2 years ago

Probably insert another Step 1: implement tests Be they simple acceptance tests, integration tests, or even unit tests for some things.

hyperman12• 2 years ago

If we're going to visit the circles of hell, let's do it properly:

Step -1: Get it under source control and backed up.

Step -2: Find out if the source code corresponds to the executable. Which of the 7 variants of the source code (if any).

Step -3: Do dark rituals over a weekend with cdparanioa to scrape the source code from the bunch of scratched cd's found in someone's bottom drawer. Bonus point if said person died last week, and other eldritch horrors lurk in that bottom drawer. Build a VM clone of the one machine still capable of compiling it.

Yes, I have scars, why do you ask?

groby_b4• 2 years ago

Step 0 sounds so easy. Until you realize __time__ exists. Then you take that away, and you find out that some compiler heuristics might not be deterministic.

Then you discover -frandom-seed - and go ballistic when you read "but it should be a different seed for each sourcefile"

Then you figure out your linker likes emitting a timestamp in object files. Then you discover /Brepro (if you're lucky enough to use lld-link.

Then you used to discover that Win7's app compat db expected a "real" timestamp, and a hash just won't do. (Thank God, that's dead now). This is usually the part where you start questioning your life choices.

Then somebody comes to your desk and asks if you can also make partial rebuilds deterministic.

On the upside, step 1 is usually quick, there will be no tests.

microtherion• 2 years ago

Get the code to build clean with -Wall.

This is fine, but I would strongly recommend against putting something like -Wall -Werror into production builds. Some of the warnings produced by compilers are opinion based, and new compiler versions may add new warnings, and suddenly the previous "clean" code is no longer accepted. If you must use -Werror, use it in debug builds.

ladberg• 2 years ago

This is fixed by the suggestion right before it:

* Reproducible builds. The sanity you save will be your own. Pro-tip: wrap your build environment with docker (or your favorite packager) so that your tooling and dependencies become both explicit and reproducable. The sanity you save will be your own.

Upgrading compiler versions shouldn't be done out-of-band with your normal PR process.

__d2• 2 years ago

That's a feature!

New warnings added to new compiler versions can identify problems that weren't previously detected. You _want_ those to -Werror when they happen, so you can fix them if they need it.

Changing a compiler version is a task that has to be resourced appropriately. Part of that is dealing with any fallout like this. Randomly updating your compiler is just asking for trouble.

nanolith• 2 years ago

The compiler and toolchain is a dependency like any other. Upgrading to a new compiler version is an engineering task just like upgrading a library version. It must be managed as such. If this leads to new errors, then this becomes part of the upgrade management. Likewise, since the code generator and optimizers have changed, this upgrade must be tested like any other new feature or fix. Create an appropriate topic/feature branch, and dig in.

planede4• 2 years ago

IMO there is absolutely no reason to enable warnings in CI without -Werror. Nobody reads the logs of a successful build.

If some warnings are flaky then disable them specifically. In my experience most warnings in -Wall are OK and you can suppress the rare false positives in code. Don't suppress without a comment.

edit:

Having said that there are entirely valid reasons to not have -Werror outside of CI. It should be absolutely disabled by default if you distribute source.

bluGill16• 2 years ago

-Wall and -Werror should be running on all developer machines and your CI machines.

If you are delivering source code to someone else (or getting source code from someone else that you build but do not otherwise work on then you should ensure warnings are disabled for those builds. However all developers and your CI system still needs to run with all warnings possible.

The days when compilers put in warnings that were of questionable value are mostly gone. Today if you see a warning from your compiler it is almost always right.

golergka• 2 years ago

Great advice. Almost all of it applies to any programming language.

dheera• 2 years ago

wrap your build environment with docker

For microservices it is fine, but you can't always deploy everything else with docker, especially for people who want to use your app inside a docker. Docker-in-docker is a situation that should never happen.

Containers are nice but they're a horrible way to pretend the problem doesn't exist.

Bundle all the dependencies and make sure it doesn't depend on 5 billion things being in /usr/lib and having the correct versions.

cshokie• 2 years ago

Not the OP, but I don’t think they meant that the build output is in a container. They meant that the thing you use to compile the code is in docker (and you just copy out the result). That would help ensure consistency of builds without having any effect on downstream users.

hobofan• 2 years ago

wrap your build environment with docker (or your favorite packager) so that your tooling and dependencies become both explicit and reproducable

If you want explicitness and reproducibility please don't reach for Docker. Unless you take a lot of care, you will only get the most watered down version of reproducibility with Docker probably luring you into a false sense of security. E.g. pointing to mutable image tags without integrity hashes and invoking apt-get are things you'll find in most Dockerfiles out there and both leave open a huge surface area for things to go wrong and end up in slightly different states.

And while they are not that easy to pick up, solutions like Bazel and Nix will give you a lot better foundation to stand on.

basicallybones• 2 years ago

A nice middle ground is using a tool like Google's Skaffold, which provides "Bazel-like" capabilities for composing Docker images and tagging them based on a number of strategies, including file manifests. In my case, I also use build args to explicitly set versions of external dependencies. I also pull external images and build base images with upgraded versions once, then re-tag them in my private repository, which is an easy-to-implement mechanism for reproducibility.

While I am in a Typescript environment with this setup at the moment, my personal experience that Skaffold with Docker has a lighter implementation and maintenance overhead than Bazel. (You also get the added benefit of easy deployment and automatic rebuilds.)

I quite liked using Bazel in a small Golang monorepo, but I ran into pain when trying to do things like include third-party pre-compiled binaries in the Docker builds, because of the unusual build rules convention. The advantage of Skaffold is it provides a thin build/tag/deploy/verify layer over Docker and other container types. Might be worth a look!

Kudos to the Google team building it! https://skaffold.dev

hawk_• 2 years ago

If you want explicitness and reproducibility please don't reach for Docker

Is it common in C++ builds to rely on the current O/S libraries instead of say making most dependencies explicit, close to full cross-compile? Do dependencies need to be pulled in using apt-get and not something like maven?

bluGill16• 2 years ago

I have docker as part of my reproducibility builds. We recently ran into a problem trying to rebuild some old code - turns out the ssl certificates in our older images has expired and so now the code isn't reproducible anyway. One more reason to agree with the idea that docker shouldn't be used.

Though we use docker for a different reason: we are building on linux, targeting linux. It is really easy to use the host system headers or library instead of the target versions of the same - and 99% of the time you will get away with it as they are the same. Then several years later you upgrade your host linux machine and can't figure out why a build that used to work isn't (and since 99% of the stuff is the same this can be months before you test that one exception and then it crashes). Docker ensures we don't install any host headers, or libraries except what is needed for the compiler.

SJC_Hacker2• 2 years ago

If you want explicitness and reproducibility please don't reach for Docker. Unless you take a lot of care, you will only get the most watered down version of reproducibility with Docker probably luring you into a false sense of security. E.g. pointing to mutable image tags without integrity hashes and invoking apt-get are things you'll find in most Dockerfiles out there and both leave open a huge surface area for things to go wrong and end up in slightly different states.

If this is frequently a problem you're doing something wrong, or using such a crappy external library/toolchain that breaks frequently on the same version.

Docker is a way to ensure that the software builds with "the most recent minor version" of some OS/toolchain/libraries.

The reason why you want the most recent version is because of security fixes and bugs.

I agree that you should check integrtiy hashes where appropriate, if you really want to fix versions.

Tainnor• 2 years ago

Not everything is all or nothing. 80-90% reproducible builds are often good enough and learning Nix only to get that last 10-20% is not always worth it. And there are ways of pinning all dependencies with docker if you really want to.

I've had issues with Dockerfiles not building anymore due to changes in the package registry, but it was like 1-2 times out of 1000 of times I used docker.

vlovich1232• 2 years ago

Step 0: CI on every commit to build & run all tests / enforce that you never regress once you've finished a step.

sega_sai• 2 years ago

To be honest a lot of recommendations apply to other languages as well. I.e. start with tests only then change, add autoformatting etc. At least I had experience of applying a similar sequence of steps to a python package.

Merik• 2 years ago

if you have access to Gemini Pro 1.5 you could put the whole code base into the context and start asking questions about architecture, style, potential pain paints etc.

jujube3• 2 years ago

Look, I'm not saying you should rewrite it in Rust.

But you should rewrite it in Rust.

elzbardico• 2 years ago

1990 Windows C++ code? Consider euthanasia as a painless solution.

codelobe2• 2 years ago

My first thing is usually:

    #0: Replace the custom/proprietary Hashmap implementation with the STL version.

Once upon a time, C++ academics brow beat the lot of us into accepting Red-Black-Tree as the only Map implementation, arguing (in good faith yet from ignorance) that the "Big O" (an orgasm joke, besides others) worst case scenario (Oops, pregnancy) categorized Hash Map as O(n) on insert, etc. due to naieve implementations frequently placing hash colliding keys in a bucket via linked list or elsewise iterating to other "adjacent" buckets. Point being: The One True Objective Standard of "benchmark or die" was not considered, i.e., the average case is obviously the best deciding factor -- or, as Spock simply logic'd it, "The needs of the many outweigh the needs of the few".

Thus, it came to pass that STL was missing its Hashmap implementation; And since it is typically trivial (or a non issue) to avoid "worst case scenario" (of Waat? A Preggers Table Bucket?), e.g., use of iterative re-mapping of the hashmap. So it was that many "legacy" codebases built their own Hashmap implementations to get at that (academically forbidden) effective/average case insert/access/etc. sweet spot of constant time "O(1)" [emphasis on the scare quotes: benchmark it and see -- there is no real measure of the algo otherwise, riiight?]. Therefore, the affore-prophesied fracturing of the collections APIs via the STL's failure to fill the niche that a Hashmap would inevitably have to occupy came to pass -- Who could have forseen this?!

What is done is done. The upshot is: One can typically familiarize oneself with a legacy codebase whilst paying lip service to "future maintainability" by (albeit usually needless) replacing of custom Hashmap implementations with the one that the C++ standards body eventually accepted into the codebase despite the initial "academic" protesting too much via "Big O" notation (which is demonstrably a sex-humor-based system meant to be of little use in practical/average case world that we live in). Yes, once again the apprentice has been made the butt of the joke.

bluGill16• 2 years ago

In the mid 1990s when C++ was getting std::map and the other containers CPU caches were not a big deal. Average case was the correct thing to optimize for. These days CPU caches are a big deal and so your average case is typically dominated by CPU cache miss pipeline stalls. This means for most work you need different data structures. The world is still catching up to what this means.

codelobe2• 2 years ago

Well, Red-Black algos are supposed to be better at cache-locality, but I have an AVL-tree impl (ugg jokes, again: AVUL (ALV) is the "evil" tree of "forbidden" {carnal?} wisdom from The Garden of Eden, associated with Yggdrasil/Odin [a "pagan" God of Balance & Pleasure]) that has improved cache locality since its data nodes can be made to contain AvlTreeNode structure(s), and avoid copying any data, as users are made to provide node alloc/free function pointers to this C lib's Tree "constructor". This means, for real example, I have a command line option interpreter with const structures for each option, each node added to two AVL trees (to find by unicode codepoint and find by length prefixed unicode string name). C++ STL Map implementations can not conditionally generate code for const types and thus do needless coppies, whereas my C collections API causes 0 calls to malloc (vs STL's 2 mallocs per node insert). NodeAlloc is just pointer math to get at the apt AvlNode, NodeFree is NoOp.

Benchmarking the STL vs my AVL approach results in millions of times quicker cmd line opt interpretaion (for my gnu getopt replacement lib) due to reduction of pointer chasing...

And if I want to do something similar in C++ (overloading operator new), I have to instantiate multiple copies of the Tree code, one per each "class". What if I want to use my Sortable class with various allocators: OBJ cache, dynamic GC'd, static (no alloc, its in the .data section already)...? Well then I get N copies of EXACT SAME template code for no real reason, only differing in delete and new [con|de]structors. The cache-misses galore this causes isn't even fair to bench against the C w/ fn() ptr approach.

sgerenser2• 2 years ago

It’s unfortunate that the hashmap picked by the standards comittee (std::unordered_map) is both awkwardly named and not very performant. Still probably better than whatever was hacked up in 1998, but nowadays you can do much better for any case where performance actually mattered. Note, still don’t roll your own, but there’s plenty of options from e.g. Abseil or Facebook’s Folly.

I worked on a project a few years ago where all data was stored in hashmaps. Just swapping out std::unordered map for an optimized implementation of a robin hood hash map increased performance by something like 2x and cut memory usage in half on many larger test cases.

samatman• 2 years ago

that the "Big O" (an orgasm joke, besides others)
worst case scenario (Oops, pregnancy)
(of Waat? A Preggers Table Bucket?)
"Big O" notation (which is demonstrably a sex-humor-based system

This post reeks of obesity, desperation, poor life choices, and old-fashioned body odor.

jcarrano• 2 years ago

Good points, but this is not something you can solve with a recipe. Investigate, talk to people and make sure you are solving actual problems and prioritizing the right tasks.

This is an extremely crucial step that you must do first: familiarize yourself with the system, its uses and the reasons it works like it does. Most things will be there for a reason, even if not written to the highest standard. Other parts might at first sight seem very problematic yet be only minor issues.

Be careful with number 4 and 5. Do not rush to fix or rewrite things just because they look like they can be improved. If it is not causing issues and it is not central to the system, better spend your resources somewhere else.

Get the team to adopt good practices, both in the actual code and in the process. Observe the team and how they work and address the worst issues first, but do not overwhelm them. They may not even be aware of their inefficiencies (e.g. they might consider complete rebuilds as something normal to do).

bombcar• 2 years ago

I did not find Chesterton's fence, and was sad.

The very first thing to do with a new codebase is don't touch anything until you understand it, and then don't touch anything until you realize how mistaken your understanding was.

bobnamob• 2 years ago

This article (and admittedly most comments here) doesn't emphasize the value of a comprehensive e2e test suite enough.

So much talk about change and large LoC deltas without capturing the expected behavior of the system first

Kon-Peki• 2 years ago

The article doesn't mention anything about global variables, but reducing/eliminating them would be a high priority for me.

The approach I've taken is, when you do work on a function and find that it uses a global variable, try to add the GV as a function parameter (and update the calling sites). Even if it's just a pointer to the global variable, you now have another function that is more easily testable. Eventually you can get to the point where the GV can be trivially changed to a local variable somewhere appropriate.

joshmarinacci• 2 years ago

You need to install linters and formatters and security checkers. But you need to start using them incrementally. Trying to fix all the issues found at once is a quick recipe for madness. I suggest using clang-tidy with a meta-linter like Trunk Check

docs:

https://docs.trunk.io/check/configuration/configuring-existi...)

dureuill• 2 years ago

What do you do now?

Look for another job

You’d be amazed at how many C++ codebase in the wild that are a core part of a successful product earning millions and they basically do not compile.

Wow I really hope this is hyperbole. I feel like I was lucky to work on a codebase that had CI to test on multiple computers with WError

throwaway71271• 2 years ago

Wow I really hope this is hyperbole.

I am sure its not, I dont have much experience as I have worked in only 3 companies in the last 25 years, but so far I have found no relation between code quality and company earnings.

throwaway20375• 2 years ago

so far I have found no relation between code quality and company earnings.

This! What matters is the market fit and customer experience. You can deliver a lot of value with average programmers working on a shitty code base.

delta_p_delta_x2• 2 years ago

So what do I recommend? Well, the good old git submodules and compiling from source approach.=

It is strange that the author complains so much about automating BOMs, package versioning, dependency sources, etc, and then proceeds to suggest git submodules as superior to package managers.

The author needs to try vcpkg before making these criticisms; almost all of these are straightforwardly satisfied with vcpkg, barring a few sharp edges (updating dependencies is a little harder than with git submodules, but that's IMO a feature and not a bug—dependencies are built in individual sandboxes which are then installed to a specified directory. vcpkg can set internal repositories as the registry instead of the official one, thus maintaining the 'vendored in' aspect. vcpkg can chainload toolchains to compile everything with a fixed set of flags, and allows users to specify per-port customisations.

These are useful abstractions and it's why package managers are so popular, rather than having everyone deal with veritable bedsheets' worth of strings containing compile flags, macros, warnings, etc.

vijucat• 2 years ago

Not mentioned were code comprehension tools / techniques:

I used to use a tool called Source Navigator (written in Tcl/tk!) that was great at indexing code bases. You could then check the Call Hierarchy of the current method, for example, then use that to make UML Sequence Diagrams. A similar one called Source Insight shown below [1].

And oh, notes. Writing as if you're teaching someone is key.

Over the years, I got quite good at comprehending code, even code written by an entire team over years. For a brief period, I was the only person actively supporting and developing an algorithmic trading code base in Java that traded ~$200m per day on 4 or 5 exchanges. I had 35 MB of documentation on that, lol. Loved the responsibility (ignoring the key man risk :|). Honestly, there's a lot of overengineering and redundancy in most large code bases.

[1] References in "Source Insight" https://d4.alternativeto.net/6S4rr6_0rutCUWnpHNhVq7HMs8GTBs6...

awesomeMilou• 2 years ago

I used to use a tool called Source Navigator

I can't believe I'm finding someone in the wild that also has used Source Navigator.

My university forced this artifact on me in the computer architecture course because it has some arcane feature set + support for an ARM emulator that isn't found elsewhere. We used it for bare metal ARM assembly programming

FpUser2• 2 years ago

And I have at some point inherited 5000+ files long legacy PHP code. I had to write Python program to parse that insanity to look for particular patterns and report those for manual fixing or do it automatically if possible. The example would be database access. That single software used 5 different methods to access it.

So no. I would not call C++ any special in this regards.

pvarangot• 2 years ago

Besides what everyone else told you make sure you are making at least 250k/y

hgs3• 2 years ago

Rewriting is questionable. Joel Spolsky has a famous blog post about this from two decades ago that's still relevant today [1].

[1] https://www.joelonsoftware.com/2000/04/06/things-you-should-...

geoelectric• 2 years ago

If the blog author is lurking on here, I tried to bookmark the article since it's very relevant to my current situation, but it didn't have a title set for the page. NBD to copy/paste one in, but it took me by surprise.

EvgeniyZh• 2 years ago

RiiR

w10-1• 2 years ago

Once you have the SCM in order, and before you make any changes:

Structure101 is the best way to grok the architecture er tangles of a large code base. They have a trial period that would give you the overview, but their refactoring support is fantastic (in Java at least).

https://structure101.com/products/workspace/

summerlight• 2 years ago

This thread has lots of good advice. I'll add some of mine, not limited to C/C++. If you have luxury of using VCS, make a full use of its value. Many teams only use it as a tool merely for collaboration. VCS can be more than that. Pull the history then build a simple database. It doesn't have to be an RDB (it's helpful though); a simple JSON file or even a spreadsheet file is a good starter. There are so many valuable information to be fetched with just a simple data driven approach, almost immediately.

  * You can find out the most relevant files/functions for your upcoming works. If some functions/files have been frequently changed, then it's going to be the hot spot for your works. Focus on them to improve your quality of life. If you want to introduce unit tests? Then focus on the hot spot. Suffer from lots of merge conflicts? The same.
  * You can also figure out correlation among the project and its source files. Some seemingly distant files are frequently changed together? Those might suggest an implicit structure that not might be clear from the code itself. This kind of information from external contexts can be useful to understand the bird's eye view.
  * Real ownership models of each module can be inferred from the history. Having a clear ownership model helps, especially if you want to introduce some form of code review. If some code/data/module seems to have unclear ownership? That might be a signal for refactoring needs.
  * Specific to C/C++ contexts, build time improvements could be focused on important modules, in a data driven way. Incremental build time matters a lot. Break down frequently changed modules rather than blindly removing dependencies on random files. You can even combine this with header dependency to score the module with the real build time impact.

There could be so many other things if you can integrate other development tools with VCS. In the era of LLM, I guess we can even try to feed the project history and metadata to the model and ask for some interesting insights, though I haven't tried this. It might need some dedicated model engineering if we want to do this without a huge context window but my guts tell that this should be something worth try.

bostonvaulter2• 2 years ago

Nice ideas! Do you have any tips for software to help automate some of those analyses?

cloudhan• 2 years ago

You run or your code runs, choose one and choose it wisely ;)

girafffe_i3• 2 years ago

Rewrite it in Rust.

girafffe_i3• 2 years ago

Lol no one reads, just RIIR.

throw_m239339• 2 years ago

I quit. Life is short.

ilitirit• 2 years ago

I've had to do this several times in the past. Honestly, my best advice would probably be make several backups, then to do as little as possible. If you need to make a small change, fine. Bigger changes? Consider if you can't do the bulk of the work in a technology or stack you understand and only make a small change to the legacy code base.

Most of the time I spend with C++ code revolves around figuring out compile/link errors. Heaven forbid you need to deal with non-portable `make` files that for some reason work on the old box, but not yours... Oh, and I hope you have a ton of spare space because of some reason building a 500k exe takes 4GB.

Keep in mind, this advice only applies to inherited C++ code bases. If you've written your own or are working on an actively maintained project these are non-issues. Sort-of.

BlueTemplar• 2 years ago

If you’re not doing [CI] already as a developer, I don’t think you really have entered the 21st century yet.

Ah yes, nothing like a veiled insult towards the people that might need that advice the most, blaming them for not using a developer paradigm that you even fail to name and for which you only give a hard to (directly) search two-letter acronym ! (/s)

( CI stands for Continuous Integration :

https://en.wikipedia.org/ wiki/Continuous_integration )

rurban• 2 years ago

I just went this very same dance with an old project, smart, which evaluates string matching algorithms. Faster strstr(). From 2013. It was in a better shape than zlib, but still.

Their shell build script was called makefile, kid me not. So first create a proper dependency management: GNUmakefile. A BSD makefile would have been prettier, but not many are used to this. dos2unix, chmod -x `find . -name *.c -o name *\.h`, clang-format -i All in seperate commits.

Turns out there was a .h file not a header, but some custom list of algorithm states, broken by fmt. Dontg do that. Either keep it a header file, or rename it to .lst or such.

Fix all the warnings, hundreds. Check with sanitizers. Check the tests, disable broken algorithms, and mark them as such.

Improve the codebase. There are lots of hints of thought about features. write them. Simplify the state handling. Improve the tests.

Add make check lint. Check all the linter warnings.

Add a CI. Starting with Linux, Windows mingw, macos and aarch64. Turns out the code is Linux x64 only, ha. Make it compat with sse checks, windows quirks.

Waiting for GH actions suck, write Dockerfiles and qemu drivers into your makefile. Maybe automake would have been a better idea after all. Or even proper autoconf.

Find the missing algorithms described elsewhere. Add them. Check their limitations.

Reproducible builds? Not for this one, sorry. This is luxury. Rather check clang-tidy, and add fuzzing.

https://github.com/rurban/smart

Jean-Papoulos• 2 years ago

A lot of this assumes the codebase is testable, but most of those legacy applications rely on global state a lot...

dirkc• 2 years ago

My take:

1. Source control

2. Reproducible builds

3. Learn the functionality

4. ...

If you don't understand what the code does, you're probably going to regret any changes you make before step 3!

jxramos2• 2 years ago

I like to use cppdepend to navigate a large and unfamiliar codebase https://www.cppdepend.com. The interactive dependency graph and integration to the editor to jump back and forth in diagrams to actual code and the many logical constructs in the source certainly accelerates getting a quick sense of the layering and a bit of the architecture of the project.

t43562• 2 years ago

I think the very best thing one can do is reduce the amount of variation you have to support. The burden of change is thus vastly reduced and the number of possible avenues for improvement explodes.

We could have left customers with old operating systems on the older versions of the product. A lot of them never upgraded anyhow. We absolutely destroyed our productivity by not making this kind of decision. We also really hurt ourselves by supporting Windows - as soon as there are 2 or more completely different compilers things turn to **t. I'm not even sure we made much money from it.

Given the ability to use new tools (clang, gcc and others) that are only available on newer operating systems we could have done amazing things. All those address sanitizers etc would have been wonderful and I would like to have done some automated refactoring which I know clang has tools for.

Most of the problems were just with understanding the minds of the developers - they were doing something difficult and at a level of complexity that somewhat overmatched the problem most of the time but the complexity was there to handle the edge cases. I wanted to go around adding comments to the files and classes as I understood bits of it. I was working with one of the original developers who was of course not at all interested in anyone understanding it or making it clearer and this kind of effort tended to get shot down.

If you don't have good tests you're dead in the water. I have twice inherited python projects without tests at all and those were a complete nightmare until I added some. One was a long running build process in which unit tests were only partially helpful. Until I came up with a fake android source tree that could build in under a minute I was extremely handicapped. Once I had that everything started to get much better.

My favorite game ... is an open source C++ thing called warzone2100 - no tests. It's not easy to make changes with confidence. I imagine to myself that one day my contribution will be to add some. The problem is that I cannot imagine the current developers taking all that kindly to it. Some people get to competence in a codebase and leave it at that.

legacybob• 2 years ago

Every morning I wake up in my legacy bed, before taking breakfast using a legacy coffee cup. I then take a shower using - you guessed it - legacy shower taps (after all, I do live in a legacy building).

I then sit on my legacy chair to browse the Internet and read about brand new programming things (through a legacy monitor).

xchip• 2 years ago

Write unit tests to make sure your refactoring work.

It better, change job to do something more interesting

nottorp• 2 years ago

Most people resort to using the system package manager, it’s easy to notice because their README looks like this:

... and goes on against using system packages.

Well if you're not using what your OS provides, why don't you statically link?

After all, it's the customer who pays for the extra storage and ram requirements, not you.

asah• 2 years ago

I love how HNers assume there are automated tests with any amount of test coverage. So cute!

...but grandpa, how did you know your code worked? Son, we didn't. <silence>

(wait until they hear that source control wasn't used...)

Dowwie• 2 years ago

Immediately rewrite everything in Rust.

m_a_g• 2 years ago

I don't want to be that person, but I'd change teams or move to a different company.

happyweasel• 2 years ago

If you have a codebase with lots and lots of tests, you are not in a bad place. Remember legacy means a codebase that works and solved and still solves problems over decades. In a sense,a successfull software project implies it will be marked as legacy. Always prefer legacy over Hype.

dieortin4• 2 years ago

A software project being successful doesn’t make the experience of working on it any better. I’d prefer hype if that means I get to avoid suffering with a three decade old C++ codebase, even if it’s not as successful.

JoeAltmaier• 2 years ago

Read it. A little every day until you've passed your eyes over all of it.

Make notes about mysterious things. Check them off once you've figured them out.

Try to find the 'business case' database. You will fail, nobody has one. Make one then, while you're reading the code.

cesaref• 2 years ago

I think the approach suggested in 'Working effectively with Legacy Code' (https://www.oreilly.com/library/view/working-effectively-wit...) is the right one. It's all about testing, and the confidence to make changes.

So buy the book, read it, apply the ideas.

btbuildem• 2 years ago

Get the build working on your machine

Nope. Make a portable/virtualized env, and make it build there. That way it'll build on you machine, in the CI pipeline, on your co-worker's machine, etc etc.

linters, fuzzing, auto-formatting

No, for at least two reasons:

1) Too risky, you change some "cosmetic" things and something will quietly shift in the depths, only to surface at the worst possible time

2) Stylistic refactors will bury the subtle footprints of the past contributors - these you will need as you walk through the tunnels on your forensic missions to understand Wat The Fuk and Why

Generally, touch as little as possible, focus on what adds value (actual value, like, sales dollars). The teetering jenga tower of legacy code is best not approached lightly.

Work with PMs to understand the roadmap, talk to sales and support to understand what features are most used and valuable. The code is just an artifact, a record of unhappy accidents and teeth-grinding compromises. Perhaps there's a way to walk away from it altogether.

dieortin4• 2 years ago

Liters and fuzzing are not about cosmetic things.

rayiner• 2 years ago

This is excellent advice, especially the list of what not to do. I don’t think it’s just C++, it’s just C++, it’s working with any legacy code base. You gotta approach it on its own terms, and analyze and fully understand what’s happening before you start changing things.

I observed from afar when the Gwydion Dylan folks (the Dylan successor to the CMU CL compiler) inherited Harlequin’s Dylan compiler and IDE and decided to switch to that going forward: https://opendylan.org. The work (done out in the open in public mailing lists and IRC) is a very nicely done case study in taking a large existing code base developed by someone else, studying it, and refactoring it to bring it incrementally into the present. They started with retooling the build system and documenting the internals. Then over time they addressed major pain points, like creating an LLVM backend to avoid the need to maintain custom code generators.

cljacoby• 2 years ago

Despite being framed as something for legacy C/C++ codebases, this is pretty good advice for setting up testing and CI automation around any project.

I recently started on a new Rust project, and despite not having to worry about things like sanitizers as much, I followed a similar approach of getting it to compile locally, getting it compile in a docker container, setup automated CI/CD against all PRs.

Although I would order the steps as 1, 3, 4, 2. Don't get out the chainsaw until you have CI/CD tests evaluating your code changes as you go.

bfrog• 2 years ago

How legacy we talking? Needs turbo C++ legacy?

victorronin• 2 years ago

https://medium.com/@victor.ronin/hn-inherited-the-worst-code...

AtlasBarfed• 2 years ago

1) Make sure you're getting paid either exorbitantly well, or at least hourly

2) Ask how long it would take to write from scratch? Only ask if the answer to #1 is "salaried well" btw.

Dumping an unsupported codebase on a new employee is a major dick move, and it requires hazard pay. They are doing that because they are desperate.

Cloudef• 2 years ago

1. Setup nix flake 2. Replace the build system with build.zig

You've just inherited a legacy C++ codebase, now what?

Related Stories