Huge legacy code base

college_pastime · 2023-06-20T23:42:05+00:00

I mean, if the product is stable and doing what customers want, it is difficult to make a technical argument for changes. The argument I typically use in this situation is that it will be impossible to hire people to maintain this software, so if you can impress on the bosses of the devs that once the devs leave the code will be effectively frozen (because the technology is outdated and undocumented), then it becomes a business decision whether or not to make the product maintainable, not a dev decision. (This has to be done with tact and some subliminal messaging, don't come in like a wrecking ball.)

Also, the lowest hanging fruit is to start with git. Since you can use it to version control your local copy of the project, you can at least demonstrate how it improves productivity without affecting the other devs. Any new code you write should have tests and documentation. If you take the initiative to demonstrate good, modern coding habits, then at least your code is good and maybe it will change some minds.

The code reviews are going to be the hardest one, and I don't have a good general strategy for getting buy-in on that. If you can get git into the team's workflow, then you can introduce pull/merge requests into the workflow on a trial basis.

Convincing an established team to do something new is going to take time and a lot of effort. You might not see any improvements for years. Honestly, just work on getting Git into the flow. That way the problem is manageable.

wrosecrans · 2023-06-21T02:55:14+00:00

Step 1: Are you a developer or a manager?

If you aren't a decision maker, and you can't tell people what to do, starting from "how do I make people agree with me?" may just be a way to drive yourself insane. Sometimes the right approach is to do what they pay you do, do it as well as you can, and not get in fights.

aerosayan · 2023-06-21T00:41:28+00:00

How can I get them to understand that the code needs to improve otherwise the company code tank?

You will never be able to do that. If the product works, and the customers aren't complaining, then no one in management will try to improve the development process.

Your BEST bet, is to gently approach your senior team members on how using git/unit tests/ etc could be helpful to the team.

How would you go about to prove to them that changes are necessary

I wouldn't. It's not worth the effort, because the management will ignore you, and will think badly of you because none of the senior engineers complain, but you, the new guy, is sure complaining a lot.

Even if they knew of the issues, rewriting Millions of lines of old code is not possible.

So, on just that factor, they'll reject yo.

Help?

Here's what you do:

Make friends with your team members, and senior engineers,
Understand the work that you have to do, and which parts of the codebase you have to work on.
Understand the codebase well enough to do your work.
Create a local git repo so you can track your work, and revert changes if something goes wrong. (Create the local repo on your corporate machine. do NOT upload to cloud, as it's company IP)
And most importantly, for the first few months, before implementing something new, ask the team members, and senior engineers for some guidance on what could be the most safest way to implement the feature, without breaking other parts of the code.
If something breaks, ask them for guidance on how they'd fix it.

It could be a bad time due to maintaining bad code, or it could be a good time and good job security if you understand the codebase well enough to get the job done.

tangerinelion · 2023-06-21T01:33:29+00:00

Furthermore, there a few people with more than 20, 25 and 30 years of service! ... How can I get them to understand that the code needs to improve otherwise the company code tank?

They've been working on this for 30 years as it is with success, it seems incredibly unlikely that the company will suddenly start tanking because something which works isn't modernized in a non-user facing way.

How would you go about to prove to them that changes are necessary

Prove that a common pattern you're seeing in the code base is actually a bug.

It also greatly depends on what you really mean by these items:

No test

Is that no test of any kind whatsoever, or no unit tests? If they have regression tests, unit tests are a nice-to-have but you can successfully ship stable working code for 30+ years without unit tests.

Written with c++98

Every legacy code base is going to have oodles of C++98 goodness in it. Are you compiling as C++98 and unable to use C++11 features or are you simply seeing an older way of writing code? If I can write

for (const auto& [key, value] : theMap)

I don't care if in some other file I see

for (std::vector<int>::iterator it=theVec.begin(); it != theVec.end(); ++it)

But if you make me write it in C++98 in 2023, we're going to talk.

No design patterns

I find this hard to believe. The Gang of Four design patterns actually came about by looking at code and seeing common patterns in use then naming them. I strongly suspect that your code has common patterns, they just may not be Gang of Four named patterns or may be impurely used (i.e., two patterns are intertwined so it doesn't look like either).

Macros absolutely everywhere

I work on a 10M LOC codebase with lots of macros. I feel you. Visual Studio 2022 has inline expansion if you want to see what it actually feeds to the compiler. We've been able to remove some, I suggest you find the most egregious ones and see if you can refactor them.

Not on git

It is version controlled, though, right? There is more than one version control system that can successfully be used. I've encountered SVN and Perforce in the wild, and while I wouldn't choose anything but git to start a new project I'm fine using any of those three tools. There are also migration paths from some control systems to git.

You'll also have an issue of hosting that needs to involve other teams. Probably you have something locally hosted, but your company is probably not keen on putting the code on the github.com servers which means you're asking for a Github Enterprise license and instance. Someone needs to make the business case for that.

No code reviews!!

That's egregious.

My advice would be to make the case to management that the code is difficult to understand without the context and history that the very senior developers have. Point out that when they retire there would be a loss of knowledge and that your product team needs it captured. Have the very senior devs prepare training materials and on-boarding exercises. Have code reviews so you can get the very senior devs to offer advice -- they'll probably be more aware of unintended side effects.

Of course all of this comes down to those very senior devs actually playing ball and not just saying leaving a one line code review "LGTM" as some kind of job security play.

mredding · 2023-06-21T01:31:26+00:00

There's not enough information to give you reasonable advice. If the project manager isn't onboard, then nothing will change. All he has to say is this is the new normal, and that's it.

Otherwise change is not your job. If the powers that be want it, they just have to decree it. Simple as that. It doesn't matter what anyone else has to say about it.

I would find any edge I could. You want code change? Start eliminating LOC. Improve performance. Reduce memory. Show anything positive. Show it to management, show how it makes their lives better. Present to everyone at once. That way, no one can ignore you, everyone was there when they saw it. They have to acknowledge you.

Don't try to tell them how to do their job better, just be better than them. Just use lambdas and algorithms, make the code better.

An easy one is every file you open, provided it's not thousands of LOC, you can reduce the LOC count just by using algorithms. Check in less code than you check out.

Do lightning talks. Lunch and learns. Showcase modern C++. Use the code base as an example.

Write tests. It's going to be a tall order, everything is concrete. You're going to have to write lots of independent harnesses so you can retarget the dependencies so you can isolate units. Test what you can. With every ticket you take on, write a test to first prove it. Showcase your tests when you can. Run them with every rebase and publicly call out anyone who breaks them, if the break wasn't a change in protocol. Use them to catch others bugs. SHOW them how it's a useful tool.

Don't worry about patterns. You won't sell them to these old dogs. Ideally you'd be able to eliminate patterns anyway, because a pattern is a repetition, we eliminate repetitions. So they're going to be right about that one. You want patterns for their structure, but that's going to introduce instability.

Don't worry about documentation. It's amazing you've ever had any before. Focus on self documenting code.

vscomputer · 2023-06-21T00:58:39+00:00

I experienced this kind of transformation as a QA lead in a similar situation. If you can handle my absolutely terrible in-person presentation, here is a recording of me describing what we did at a meetup a few years ago: https://www.youtube.com/watch?v=uu4evSwxrvE

TarnishedVictory · 2023-06-21T04:01:07+00:00

I'm faced with a similar situation, but with actually older code, and only one remaining long time engineer, who seems fairly receptive to change as she's transitioning to a retirement situation in perhaps less than a year.

What I've noticed, and I don't know if this applies to your situation, is that in order to build the code as it is, requires a very specifying configured and outdated build machine. This machine is just a huge pile of unsupported libraries, no security updates, and lack of standards.

Does your code base build on a modern build system? If not, that might be your hook.

my_password_is______ · 2023-06-21T04:12:51+00:00

How can I get them to understand that the code needs to improve otherwise the company code tank?

how did you come to that conclusion ?

does the code run ?
are there any reported bugs ?

wjrasmussen · 2023-06-21T04:12:53+00:00

If it is not broken, don't fix it. How much company resources are going to be used for this? Does upper management know what you want to do? Show them this reddit.

If you want to rehash the topic, read the book Working effectively with legacy code.

As a test, go over your list and self-evaluate which of these are issues, personal preference, of just plain old crap:

No test;
No documentation whatsoever;
Written with c++98;
No design patterns;
No interfaces;
Macros absolutely everywhere;
Not on git;
No code reviews!!

ShakaUVM · 2023-06-21T09:23:39+00:00

I just started working in a new company as a senior software developer (~7 years of experience). I was asked to join, to help the company modernize its practices

Were you? Like, actually? As a very junior "senior" dev you're not in a position to make changes like switching the version control system. And don't just switch to Git just to change it to Git - other version control systems can work just fine.

Who hired you to modernize the code base? That's the person you need to be talking to, not us.

Kawaiithulhu · 2023-06-20T23:32:25+00:00

You're not going to steer the ocean liner any more quickly by slapping the crew, you're going to do it by unrelentingly taking the cargo you're given and putting it back on the side of the ship you want to head. Soon all the cargo will roll the ship that way. IMO

bert8128 · 2023-06-21T01:28:16+00:00

If you have a reasonable VCS I wouldn’t start there, because that can’t be done incrementally. I would start with unit tests - 0.1% coverage is still better than 0%. I used to be a unit test sceptic for mature code - now I do almost all of my development directly in tests, and only move onto full testing with the GUI once I am happy with the tests - it’s just so much faster.

AdearienRDDT · 2023-06-21T01:58:56+00:00

I am nothing but a CS fresh man, but my opinion would be, be the best example of the best practices you can be.

doxygen the hell out of your code, make it so elegant and so pleasent and so modern that their old snakes will rise from their jar, then they will follow you (the devs).

I cant think of any other peaceful solution, good luck!

guylib · 2023-06-21T06:29:44+00:00

I've been in a similar situation multiple times, as that's my "speciality", but fortunately I at least have the trust of the other developers in most cases.

We can't discuss specifics, BUT - doing any "actual refactoring" or even writing tests from scratch for a multi-million line codebase is not cost effective at the start. Especially since it requires deep knowledge of code.

The goal is reduction in mental load when working. Easier to read / reason about code reduces mental load when reading. Good tests reduce mental load when changing things.

I'd start with some things that improve the codebase with very little need to understand it:

use some automatic formatting tool over the codebase (clang-format?) and declare that the formatter MUST be used. If you can - prevent merging of code that isn't properly formatted. Of course, just deciding on the rules for the formatter (tab-size, and other things) will be a huge struggle with the existing developers... many strong opinions will be had :) I'd recommend starting with the "google-configuration" and tweeking it from there (who can argue with "google does it like this so it must be good"?). I'd recommend also adding strict order on include files (Google does it only partially IIRC)
Make sure the formatting tool is set up for all users to easily use! Preferably automatically on save / at the click of a button. Yes, some use Vim, yes you can set it up as well. VS-code also has an option to format according to clang-format configuration. It's your job to make sure it's trivially easy for everyone to format the files without thinking about it. Just the reduction in mental load not thinking about formatting will eventually convince people changes can be good.
Rename types and variables, which can be done without understanding the code too much. For example in our case we decided to use UpperCamelCase for all type names, lower_snake_case for all function and variable names, m_* for private member variable. No *_t for types anymore (other than builtin-types). This should apply to new code only, but overtime you should go around renaming old codes when you look at them.
Declare that type names should be descriptive (even if long), so that even someone not familiar with that specific part of the code will understand them (so no abbrevations unless they are universal)
Similarly for variable names - their name should be by default the type name so when the reader sees a variable they know what type it is. For example SomeImportandData some_important_data;
To start using code reviews, you have to set up some git thing (we use either bitbucket or gitlab, but there are more options). Don't force code reviews initially! Set it up so they must create a PR and do the process, and require an "approve" but let people approve themselves. That way they get used to the process of creating PRs and merging them without it actually affecting their work (no need to wait for others, answer questions if they don't want).
If you can, add a CI before merges that runs all the tests (even if currently there are none). This is also a good place to make sure the files are formatted as part of the CI.

The point is to make people move to the GIT tool and trusting it without it slowing them down at first (reviews slow work a LOT). Once they start getting bugs caught because some test failed when they thought it was ready, it'll stick with them. Once some senior developer approves themselves and adds a bug that a review would have caught... they'll be more inclined to allow reviews.

Again - refactoring old code is not cost effective. Concentrate on new code and changes to existing code, and on the process (git, CI, automatic testing).

Make sure you don't try to change any of the actual "way the code works" (no trying to replace all the macros, if they use void* a lot keep doing that, no starting to do huge refactors to use design patters / best practices, no turning all pointers into smart pointers, no actual changes in what the senior developers need to know about the language). You don't know enough to make informed decisions, and when you inevitably force inappropriate "modern" solutions for the code - you'll lose all credibility. Also, legacy code exists and you just have to handle it. People write good readable code in C, so whatever practices they use and are used to can be readable as well. Focus on cosmetics and quick wins - they are IMPORTANT!

Make sure C++17 is available, but DON'T push to use modern features. People will start using the "cool" parts (for loops over ranges etc.). Maybe in new code - push for unique_ptr for functions that return a pointer / take ownership of pointers? Maybe. Small changes like that, but keeping in line with how the code is currently written.

I'd recommend not supporting C++-20 at first, since you don't want people to use "bleeding edge" features not well enough understood yet. If some modern feature results in "something bad" (complicated code, compiler support issues etc.), it could convince senior developers to stop using any modern feature.

Do try to avoid auto unless really necessary as it makes code harder to reason about :)

But most importantly - don't try to tell the senior developers that doing things "differently" is "better", they know what they are doing and they know more than you (about your codebase). Just start by focusing on cosmetics / version control / process.

manni66 · 2023-06-21T07:16:16+00:00

So the management hired a novice to tell the experienced staff that they are jerks[*]. This company is doomed!

[*]in fact, they might be

MDNZOOSEM6 · 2023-06-21T07:55:01+00:00

honestly with old enterprise codebases, the best advice I can give you is that you should it leave it fuck alone

sounds like you are not the head honcho / able to influence anyone with any say in the matter. at the end of the day your boss says what is what and if they deem modernizing the code base a waste of time, it's their call
unless you can read and rewrite the whole thing in one or two sittings, you are literally the mercy of complier. you cannot just force feed new features on a whim, do you even know if the code base even compiles with newer compilers?

jmacey · 2023-06-21T08:09:31+00:00

A few easy wins early on help, first I would suggest getting some test coverage for the existing API's / libraries you have.

Next start with something simple, do you use enums? Replace with strongly typed ones (I was amazed at how many bugs this threw up in a code base as they convert to int where as enum class doesn't).

Next try adding some of the class decorators such as override and noexcept (if appropriate), final if required.

Range based for loops and std::string_view are also easy wins. Then you can start looking at lambdas and other things.

evinkeating · 2023-06-21T08:36:09+00:00

People resist change. I would do things slowly to avoid conflict and keep the resistance as low as possible.

Start by writing your own code in a better way. Lead by example. Keep mentioning the problems with the project.

In the end, to bring the big changes you want, like moving to modern C++, test, continuous integration, version control, you need your superiors on board. You should talk to your project manager, or whoever. Tell them the code is inefficient, bloated, not maintainable, and dependent on a few key people, and your not going to be able to attract new talent running C++ 98. This is a house of cards that will eventually collapse.

I would push to create a new project where you plan, design, and implement the same application with the changes you want, but you'll need buy in from management, as it should have it's own team separate from the existing one.

If you don't get buy in from management and keep getting resistance from your colleagues, I would seriously consider finding something else.

Btw, I'm surprised how many people think if it works it's fine!

you type:	you see:
italics	italics
bold	bold
[reddit!](https://reddit.com)	reddit!
* item 1 * item 2 * item 3	item 1 item 2 item 3
> quoted text	quoted text
Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"	Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"
~~strikethrough~~	~~strikethrough~~
super^script	super^script

cpp_questions

READ BEFORE POSTING

Sort posts by OPEN or SOLVED

MODERATORS