24 ms·
You've just inherited a legacy C++ codebase, now what?
- EvgeniyZh 3y agoRiiR
- sylware 3y ago[flagged]
- shrimp_emoji 3y agoThat just ports all the problems to C... You're better off incrementally rewriting it in modern C++. Replace those raw pointers with references or smart pointers!
- sumtechguy 3y agoAll of that #define madness replace with templates and constexpr. I would also add if you can get it running on more than one compiler. G++ and MSvc and clang. Each one has its own subtle bits of errors it likes to throw out. Also get it running properly with something like valgrind. You are going to need it.
- sylware 3y ago... but you only need a C compiler, not a grotesquely and absurdely massive and complex c++ compiler. This is all wrong.
- shrimp_emoji 3y agoWhat, are you building the compiler yourself? :x It'll be way worse than those grotesquely and absurdly massive and complex compilers.
- keepamovin 3y agoIt'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_Thastus 3y agoIME, this only works if you can get regular help from them. A one-off won't help much at all.
- keepamovin 3y agoYeah you need to cultivate those relationships. But with a willing partner that first session will take you from 0 to 1 :)
- mst 3y agoA lot of the time it takes you from 0 to 0.1, but (a) every little helps (b) if you take them out for lunch or a post-work beer or whatever it can build a relationship where you can ask follow up questions via email. Ideal is if they have a life such that something morally equivalent to "how about we meet up and I'll pay for the food/beer" is a viable thing to suggest later. Oh, and always remember - the way to a geek's schedule is often through their wife. If you get a chance to meet their partner, TAKE IT and be on your best behaviour, if $partner likes you then you have massively increased chances of making useful things happen later.
- keepamovin 3y agoYeah, that's a good point. But I think more often it's closer to 1 because (I guess this depends on our definition of 0 and 1 haha! :)) a) you get the benefit of their conceptual models for thinking about the codebase, which is hard won and highly useful, and b) you grok any practical pitfalls or things to remember when running, building, testing, that may be simple enough, but in the space of all possibilities are hard to come by without knowing them. So basically you get to go from: 0 - can do nothing at all; to 1 - being able to feel confident diving in and looking around, framed with the downloaded conceptual models, knowing how to hit the ground trotting, if not running. To me, 0.1 is more like you slugged it out for a couple hours poking around by yourself, and now you know a handful of confusing things with low confidence. Chatting to one who has been there, done that lets you have more confidence, and know more actionable stuff, which is highly useful. I suppose it depends on your conversation, listening and comprehension skills tho! Hahaha! :) But seriously not everybody learns well in that situation. I just happen to. I'd much rather talk it over with someone and absorb, than watch a video or read a tutorial. Finally -- haha! :) -- I guess there are some geeks to whose schedule the way is through their husband or whatevs haha! :)
- rwmj 3y agoSurprisingly good advice. In a similar vein, Joel's 12 steps to better software: https://www.joelonsoftware.com/2000/08/09/the-joel-test-12-steps-to-better-code/ https://www.joelonsoftware.com/2000/08/09/the-joel-test-12-s...
- leecarraher 3y agocreate bindings and externalize function libraries for other languages, hope to your prefered deity nothing breaks
- Night_Thastus 3y agoThis adds additional problems. IE, Start replacing legacy C++ with Python, now debugging and following the flow of the code becomes very difficult.
- bluGill 3y agoIf 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.
- Night_Thastus 3y agoIs it possible to integrate any of those while allowing seamless debugging? IE, step right from one into another? I've yet to see that happen.
- bluGill 3y agoIf nothing else my C++ debugger will see the functions of everything in Rust, D, or ada - it might be a mangled name but generally I can figure them out. Once you step into python the debugger is going to see the python runtime functions and you need to dig into them to figure out what of your functions you are running. I have yet to figure out how to get any language other than C++ into my system so I can't say how well it works in the real world. Then again I work on an embedded system with real time controls so I can rarely use a debugger since as soon as I hit a breakpoints all my must happen at time X functions fail to run and the whole system fails in a few ms.
- sjc02060 3y agoA 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).
- tehnub 3y agoWould you mind sharing what language you used?
- jstimpfle 3y agoProbably not a project spanning more than 3 decades of development and millions of lines of code?
- throwaway2037 3y agoDo you have a public write-up (blog post)? If yes, you should post it on HN. It would probably generate lots of interesting conversation.
- Jtsummers 3y agoI'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_ 3y agoFair point!
- dralley 3y agoOn the flip side, auto-formatting will trash your version history and impede analysis of "when and why was this line added".
- skrebbel 3y agoYou 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.
- fransje26 3y agoGood to know. Thanks for the tip!
- Jtsummers 3y agoI'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.
- ecshafer 3y agoThis 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.
- bluGill 3y agoThe 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 3y agoStart grinding leetcode and find another gig?
- bun_terminator 3y ago> Rewrite in a memory safe language? like c++11 and later?
- z_open 3y agoHow is that memory safe? Even vector out of bounds index is not memory safe.
- bun_terminator 3y agoYou can access a vector with a function that throws an exception if you so desire
- TwentyPosts 3y agoYou can also just write no code at all if you so desire. It certainly won't cause any memory issues that way. (Hint: What you yourself decide to write or refrain from writing is not the problem. You're not they only person who ever worked on this legacy codebase, and you want guarantees but default, not having to check every line of code in the entire project.)
- bun_terminator 3y agono you just have to write a githook with some static analysis, like literally everyone who does proper c++. Safety hasn't been an issue in c++ for more than a decade. It's just a made up thing by people who don't use the language but only want to hate.
- z_open 3y agoGo look at the CVEs and github issues of modern C++ codebases. Your statement is nonsense. Chromium is still plagued by use after free. How high do you set the bar? Which codebases are we allowed to look at?
- 3y ago
- sealeck 3y agorm -r Problem solved
- dtx1 3y ago> C++ > Not even once
- deleted 3y ago[deleted]
- GuB-42 3y agoIf 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".
- sealeck 3y agoI'm being like 1000% facetious, and agree that rewrites are bad.
- bluGill 3y agoI 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_Thastus 3y 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 3y agoAn 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.
- zer00eyz 3y 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.
- bluetomcat 3y agoBeen 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.
- johngossman 3y agoAbsolutely. 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.
- cratermoon 3y ago> Fix obvious bugs. Be careful about that. Hyrum's Law and all.
- johngossman 3y agoShould have been clearer. You’ve probably been put on the project because something isn’t working. Fix the simplest, most obvious of these. Fixing a bug is a good way to learn.
- cratermoon 3y ago> You’ve probably been put on the project because something isn’t working. Perhaps if it's a change requested by the organization or the users. Just don't go "fixing" things that look like bugs without knowing if it's really a bug or expected behavior.
- bluGill 3y agoIn 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.
- black_13 3y ago[dead]
- sk11001 3y agoIs 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 3y agoYes. 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.
- sk11001 3y agoMy 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.
- hilux 3y agoDid you see yesterday's article about the White House Office of the National Cyber Director (ONCD) advising developers to dump C, C++, and other languages with memory-safety issues?
- sk11001 3y agoYes, and at the same time I’m seeing ads for jobs that pay more than double what I make that require C++.
- mkipper 3y agoI still think knowing C++ is pretty valuable to someone's career (at least over the next 10 - 15 years) if they're looking to work in fields that traditionally use C++ but might be transitioning away from it. The obvious comparison is Rust. There are way more C++ jobs out there than Rust jobs. And even if I'm hiring for a team developing something in Rust, I'd generally prefer candidates with similar C++ experience and a basic understanding of Rust over candidates with a strong knowledge of Rust and no domain experience. Modern C++ and Rust aren't _that_ dissimilar, and a lot of ideas and techniques carry over from C++ to Rust. Even if the DoD recommends that contractors stop using C++ and tech / finance are moving away from it, I'd say we're still years away from the point where Rust catches up to C++ in terms of job opportunities. If your main goal is employment in a certain industry, you'll probably have an easier time getting your foot in the door with C++ than Rust. Both paths are viable but the Rust path would be much harder IMO.
- davidw 3y agoWell, tomorrow is the "who's hiring?" thread...
- tehnub 3y agoI 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_ 3y agoAuthor 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.
- tehnub 3y agoMakes sense, thanks!
- avgcorrection 3y agoSo 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 3y ago(author actually shows up to advocate for rust)
- avgcorrection 3y ago
- huqedato 3y agoWhenever 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 3y 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.
- cratermoon 3y agoKill It with Fire https://nostarch.com/kill-it-fire https://nostarch.com/kill-it-fire
- myrmidon 3y agoReally 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 3y 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.
- jpc0 3y 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.
- grandinj 3y agoThis 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 3y agoThe “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.
- jujube3 3y agoLook, I'm not saying you should rewrite it in Rust. But you should rewrite it in Rust.
- beanjuiceII 3y agothe whitehouse says i should RiiR
- jeffrallen 3y agoThis 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_chan 3y agoI’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 3y agoThe 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.
- bluGill 3y agoThe 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_chan 3y agoYes, 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.
- VyseofArcadia 3y 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/ https://xkcd.com/1172/
- Palomides 3y agohard agree removing a feature is possibly the most politically intractable thing you can try to do with a legacy codebase, almost never worth trying
- lelanthran 3y 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 3y agoHow 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!
- bluGill 3y agoVery 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".
- deleted 3y ago[deleted]
- eschneider 3y agoSome 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 3y agoI 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.
- dataflow 3y agoStep 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)
- daemin 3y agoProbably insert another Step 1: implement tests Be they simple acceptance tests, integration tests, or even unit tests for some things.
- sega_sai 3y agoTo 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 3y agoif 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.
- elzbardico 3y ago1990 Windows C++ code? Consider euthanasia as a painless solution.
- codelobe 3y agoMy 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.
- bluGill 3y agoIn 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.
- jcarrano 3y agoGood 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 3y agoI 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.
- deleted 3y ago[deleted]
- bobnamob 3y agoThis 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 3y agoThe 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 3y agoYou 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-existing-linters/clang-tidy-setup https://docs.trunk.io/check/configuration/configuring-existi...)
- dureuill 3y 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 3y 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.
- throwaway2037 3y 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.
- gmueckl 3y agoI started to joke that in order to have a successful software startup, you need to essentially write the most godawful program code you can get away with. The money is much better spent on a good/aggressive sales strategy. Elegant technology never wins on its own merits.
- delta_p_delta_x 3y 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 3y agoNot 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_0rutCUWnpHNhVq7HMs8GTBs6osGo8uUyXjqk/rs:fit:2400:2400:0/g:ce:0:0/YWJzOi8vZGlzdC9zL3NvdXJjZS1pbnNpZ2h0XzM5MTk4MV9mdWxsLnBuZw.jpg https://d4.alternativeto.net/6S4rr6_0rutCUWnpHNhVq7HMs8GTBs6...
- awesomeMilou 3y 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
- FpUser 3y agoAnd 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 3y agoBesides what everyone else told you make sure you are making at least 250k/y
- hgs3 3y agoRewriting 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-never-do-part-i/ https://www.joelonsoftware.com/2000/04/06/things-you-should-...
- geoelectric 3y agoIf 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.
- w10-1 3y agoOnce 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/ https://structure101.com/products/workspace/
- summerlight 3y agoThis 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 3y agoNice ideas! Do you have any tips for software to help automate some of those analyses?
- cloudhan 3y agoYou run or your code runs, choose one and choose it wisely ;)
- girafffe_i 3y agoRewrite it in Rust.
- girafffe_i 3y agoLol no one reads, just RIIR.
- throw_m239339 3y agoI quit. Life is short.
- ilitirit 3y agoI'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 3y 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/ https://en.wikipedia.org/ wiki/Continuous_integration )
- rurban 3y agoI 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 https://github.com/rurban/smart
- Jean-Papoulos 3y agoA lot of this assumes the codebase is testable, but most of those legacy applications rely on global state a lot...
- dirkc 3y agoMy 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!
- jxramos 3y agoI like to use cppdepend to navigate a large and unfamiliar codebase https://www.cppdepend.com 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.
- notso411 3y ago[dead]
- t43562 3y agoI 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 3y agoEvery 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 3y agoWrite unit tests to make sure your refactoring work. It better, change job to do something more interesting
- nottorp 3y 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 3y agoI 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 3y agoImmediately rewrite everything in Rust.
- m_a_g 3y agoI don't want to be that person, but I'd change teams or move to a different company.
- happyweasel 3y agoIf 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.
- dieortin 3y agoA 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 3y agoRead 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 3y agoI think the approach suggested in 'Working effectively with Legacy Code' (https://www.oreilly.com/library/view/working-effectively-with/0131177052/ 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 3y 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.
- dieortin 3y agoLiters and fuzzing are not about cosmetic things.
- rayiner 3y agoThis 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 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 3y agoDespite 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 3y agoHow legacy we talking? Needs turbo C++ legacy?
- victorronin 3y agohttps://medium.com/@victor.ronin/hn-inherited-the-worst-code-and-tech-team-i-have-ever-seen-how-to-fix-it-f06ede3fc3b5 https://medium.com/@victor.ronin/hn-inherited-the-worst-code...
- AtlasBarfed 3y ago1) 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 3y ago1. Setup nix flake 2. Replace the build system with build.zig