C++ Weekly - Ep 402 - Reviewing My 25 Year Old C++ Code (IT'S BAD!)

[@stefanhellwig-lp4rg]

Great video! I would like to see you modernize and fix this old code. I have to deal with a lot of legacy code like this every day so it would be very interesting to see what you will turn this into (probably using C++20 „only“ since C++23 is not yet so widely available?). Your comments are also very educational and helpful so a follow-up video (or more) would be great!

[@MsDuketown]

Typically I use XBMC / Kodi as a reference. It's an old codebase with professional management of over 1 million LoC. They only accepted small sets of commits, to keep the repo with the codebase diff-able. Typically you see some replacing some constructs, removing blocking stuff, applying code guideline stuff, inserting wanted behaviour, extra safety measures, extra protection measures. etc.

[@kdkrull]

I would be curious to see the old code modernized too, since the last time I wrote C++ was also in a university setting around the same time. Seeing your process would be a good way for me to start catching up myself.

[@andreialdea6072]

yes. an episode (or maybe better a stream / vod ) of you taking this code from where it was in '98 to the latest c++ standard and best practices would be appreciated, especially for the folks looking for good examples of modern c++ code ( me included )

[@QQQQQAQQQQQ]

In a way it would also be interesting to see it in two steps, fix the bugs but stay in C++98 to see how it should have been done at the time. Then step two modernizing it to C++23.

[@36Berlin]

I agree. We all would learn much more.

[@cppweekly]

Stay tuned!

[@CramBL]

Modernize it! But please also take the opportunity to also teach the process of gradually modernizing, both code and toolchain. Big challenge to gradually migrate legacy code without regressions.

[@vfmadd231pd]

That’s my second C++ weekly video and I definitely want to see a follow up on that 😂 Fix it, modernize it, benchmark it!

[@starchsky]

More please. This made me feel good about my own (not nearly as bad) C++.

[@nextlifeonearth]

Yeah, same here. Learning resources have improved by buckets in the mean time (and the language is actually standardised, compilers have improved. Overall just better)

[@Skaaaaal]

Hello, please fix it and modernize it fully. This is so instructive !

[@michaelfahey-ww3tq]

Yes please make a follow up modernization video! It would be great to see the code as is compile and then run ASAN/UBSAN to show all the memory leaks. Then modernize and show how modern c++ helps fix all of those issues older c++ is prone to!

[@antonkantsemal6216]

Yes please for the follow up. This is an essential mix of the best practices applied. An exclusive look at the work of JT in the wild!

[@GabeWachob]

This feels like mostly a great "modern C++" vs "old school C++" video. I learned C++ in college in the early 90's when it was still thought of by many as a pre-processor layer on top of C, and then swore it off as long as I could. The C++ world was very fractured and non-standard. Modern C++ is such a joy compared to old timey C++.

[@daran0815]

It wasn't just thought of being as being a pre-processor. At the time it was usually implemented as such. That's no longer possible for todays language.

[@christer8964]

Yes, make the code work without memory leaks. But modernize the code in a second video. Use what was available at that time first. Not strange that you used ".h". Almost every old C++ book used that.

[@brlvldvlsmrtnz]

Yeah I would love to seem him try to make the void* stack idea work even though it's blasphemous and a very very error prone interface

[@briansalehi]

You have to see the code my ex-company was using, all the headers were ".h" for absolutely no reason. I wish it was only the old books spreading bad practices.

[@stevesimpson5994]

Absolutely. Sometimes modernizing the tool-chain is NOT allowed.

[@F4raya]

Same, I would like to get a video with code fixed perhaps to the 98 standard. And a second video that modernizes it.

[@vorrnth8734]

And to be sure use the compilers of that era. That would be Visal C++5/6 or gcc 2.95. Both deviate in different ways from the then coming 98 standard.

[@JohnJonesJMJAtlanta]

I was proud of myself that I knew your stack implementation was because templates weren't well implemented until near the end of the 90s. Then I realized it is probably because I'm old enough to remember when templates were new. More interesting than modernizing the code is doing a [executable size|memory|speed] comparison between old and new, using the same compiler/linker.

[@cppweekly]

That's actually not the problem here - templates work perfectly fine in the contemporary compilers from the era for this use case (I just checked!)

[@nghiapham6000]

Love this content, I'm a beginner trying to learn C++, watching how professionals code and fix mistakes in linse of code help me to learn better.

[@davidesleeds]

Jason, I was so entertained by your reactions! Sadly, I have the same response to too much code I review to this day 😂

[@Heater-v1.0.0]

Great. Love to see that relic modernised, compiled and running.

[@remmyrambrandt9497]

Great episode. It would be awesome if you could fix and update your old code so one can clearly see how to repair those edge cases.

[@MrAlbinopapa]

Considering all the features added to C++ since '98, a lot of the code is going to disappear save for the core functionality, so it would be interesting to see the transformation from legacy to modern. Honestly, would prefer a video as opposed to a live stream since too many things can cause the live to run longer than needed.

[@CedLePingouin]

Ha ha. I feel your pain. Every time I have to look at old code of mine, I feel the same (and I think it's actually a good thing). Would love to see you fix and modernize that code. And maybe even try to compile the "original" version (if that's still possible) and check all the memory leaks it contains, just for fun.

[@jdvd]

Please, oh, please yes, do it! Fix and modernize that... stuff. This was hella fun!

[@icemojo]

It's always fun to go down a memory lane. Great video. Other comments have said it enough. We definitely need a follow up.

[@blablabla7796]

I would love to see you fix this code. Make it modern but I want you to keep the heart of the code the same. So don’t change the custom stack class into std::vector and keep the overall code design still the same for the most part. Then I want another video where you essentially throw away the original codebase, then re-solve the problem with modern C++ and 25 extra years of code architecture and design. So no more custom classes where there are std constructs that do the same thing and make a more extensible design. As if you’re making a codebase that other people might use and extend in the future.

[@haiphamle3582]

Great content! I would love to see more of this. It would be luxury for developer to view this kind of review session. I think for the first file, we can add some details like: The header should use some kind of guard, or pragma, the constructor should be explicit. And I so love the habit to use more `const`.

[@johnhsu5459]

Please do make a follow-up episode in which you fix/modernized this code.

[@aesthetic3777]

Nice video! Would be educative to watch you modernize your code.

[@ll-ib5jr]

15:00 Yes, modernize it in another episode! But what about modernize it by iteration throughout all standards from 98 to 23?

[@russCoding]

Hi Jason, thanks for your video series and all the effort you do to share knowledge. On the point of fixing and modernizing the code, I think it would be a great idea if coupled with feedback as to why any particular line had to be changed to fix or modernize it.

[@蜜熊胖胖猪]

Very good video! Just, for beginner, if you could give some arguments to explain why you think something is wrong, I can learn more. If you do not mind, could you make a video to talk about ownership? and when pointer, when not? when shared_ptr when not? When heap, when stack? I have read c++ primer book, but I can not really conclude of these rules. I have used c++ for 1.5 years with enterprise experience. But, we are small company, no resource for training things, and seems like my senior also can not explain it using own words.

[@SfrOschNOR]

I'm working on a big legacy C++ Project and it's nice to see why certain design decisions where made back then. For example because it did not work back then. It woud also be nice to see how it would look like now.

[@pdkama]

Nice. Would like to see it compile with old and modern compilers.

[@carlosrnardi]

Hi Jason, would be really nice to see the code completly refactored

[@Mooproxy]

Non-static member initializers (ie being able to int* member_ptr = nullptr when declaring the member) were a C++11 feature if I remember correctly, not C++98. But modernising it would be an interesting video. It's very very very easy to forget how barebones C++98 and earlier C++ was compared to modern C++. Also, void main() :P

[@antonionatilla9848]

I'd definitely watch the follow-up episode; it could be yet another learning experience!

[@theicebeardk]

I would love to see you modernize this both because it is a good way to teach folk. Also because I have been in the same situation and I can just hear the cringe in your voice when you had to look at your own ancient code and it was the same as mine I think.

[@ezekieloruven]

A LOT of people working with C++ are maintaining legacy code, so modernizing a legacy codebase from C++03 to, say, C++17 or 20 would be really neat!

[@ignabelitzky]

I would like to see you refactor this code and compile it!! Thanks and I'm glad you make this video

[@Omnifarious0]

I think you're a bit too hard on yourself. Returning a pointer to a RationalExpression is not hugely unreasonable given ideas about returning things larger than an integer prevalent at the time. And similar things were not considered bad practice at the time. It's taken the C++ community a long time to arrive at the current set of "considered good" coding idioms. Reliable template support wasn't actually available in any compiler until the early 2000s. I remember considering them a janky (in terms of compiler support) feature in the mid-90s. And I also remember not having the STL to work with. I'm not sure if this code is old enough for that, but given the absence of the STL, and templates, I don't think your Stack implementation is awful.

[@garytaverner5930]

'void main'? Surprised you didn't call that out! Was that ever correct in C++?

[@oschonrock]

Brilliant.. your pain was palpable!

[@skilz8098]

Since this project is relatively small along with the consensus coming from your viewers to modernize it with best practices; as an aside you could turn it into a little challenge by modernizing it in incremental steps. I'm referring to modernizing it from its current legacy form by updating it to C++11 then C++14, 17++, C++20 and finalize it with C++23 syntax and semantics. You could turn this into a mini series so that your viewers could see it's transformation take place in stages in reference to how the C++ language has evolved throughout the years. This can serve the community in various ways. Maybe not all who watch your series has access to the latest compiler, and at least they can use your techniques as a guide for how to update legacy code they may come across within the context of the language tools and versions that are available to them.

[@AndreasBeham]

Best quote after seeing all the allocations and pointers used: "Oh, I was learning Java at the same time". 😂🤣

[@wChris_]

I would much rather see a c++98 version, which you could have written with your current knowledge and only then modernizing it to c++23.

[@kinershah464]

Yes that would be interesting as well.

[@KhalilEstell]

Oh my goodness. This was an amazing video to watch. I'd love to see this code getting modernized!

[@browaddup3289]

please modernize the code and make it work, would also love to see you coding stuff in c++, while explaining your thought process ❤️❤️

[@stephenhowe4107]

Bug at 6:12 The new allocates space for the string but forgets to add +1 for the trailing '\0'; Next line copies the string using strcpy() and immediately clobbers the byte following by adding the '\0' (and no space to hold it) I would store strlen(n_string) in a size_t variable, then do memcpy() instead of strcpy() and populate stringlen (whuch should be size_t) In fact strlen(n_string) i should be stored in stringlen which should be size_t type

[@imtootired4122]

Also visible there, string is allocated with the array form of new, but the destructor releases string with non-array delete.

[@stephenhowe4107]

​@@imtootired4122 Good point. That is undefined behaviour. No copy constructor or assignment operator which means the compiler will manufacturer one - which will do the wrong thing,

[@cppweekly]

@@stephenhowe4107 Indeed to both! This comes up in the next episode about this topic.

[@dp.9130]

Yes we definitely want to see how you modernize, test and run this code

[@simonmaracine4721]

You may also use an enum class or enum at least instead of a bunch of defines for the tokens. A follow up video with fixing and modernizing this code would be great.

[@aurelien221]

I'd love to see the modernized and sanitized version!

[@youtubeviewer7077]

Would be cool to see you fix it but just get it compiling with "-std=c++98 -pedantic" rather going hog wild with all the adornments of modern c++ keyword/annotation soup. Basically make it what would have been good code at the time.

[@3timeszero]

Do it. MAKE IT WORK.

[@iliasalaur]

I would LOVE to see you modernise this code. Just’ve ended implementing my own infix notation(shunting yard algorithm) parser and RPN evaluator.

[@pancio-ciancio]

Typeless stack can be usefull to allocate different types. Do you think it's better to have multiple stack of the appropriate type instead? Right now I'm focusing on memory management. This seems a good project to talk about. So, please make more video on it

[@ab-jc1qf]

Yes, please do a follow up and fix/modernize the code. Thank you.

[@resonancecascade6497]

Yes, please modernize it and get it to compile. Actually this could be a whole new series. Find some old C++ code and do a review & modernization. It would be a great way to teach how to code modern C++.

[@Beatleman91]

Did they have clang-formatter in 1998? Because without that thing I still might be inconsistent with tabbing, braces, spaces and such

[@slipcurve1410]

sometimes i run into c++ that is written by people who are stuck in 90s. this feels very familiar. the wtf moments are not quite that bad, but similar. one thing that sticks out: having a "type" indicating integer (not even an enum) and a void* as function parameters. the function c-cast the void* to the type the number indicates (arbitrary, identified by macro) and then does different stuff based on the type. yeah. my expression was that of the jackie chan wtf meme.

[@stevesimpson5994]

Arrays decaying to a pointer to the first element is an unfortunate C legacy, but I don't agreed the function signature is useless. In a public header array parameters communicate intent -- to a real person. Sure the compiler will do as the rules say, but the point is that code needs to communicate to the reader. A pointer parameter by itself does not indicate that the argument is intended to be a contiguous block of memory. It could for instance be nullptr.

[@anon_y_mousse]

I agree with that. The function signature should relay intent. That's why C has keywords like restrict and C++ has nodiscard as an attribute, which C23 will also be getting, to name a few.

[@ABaumstumpf]

@@anon_y_mousse "That's why C has keywords like restrict" That is a keyword and functionality that C++ really needs but the committee is refusing to adopt. Cause not only does it communicate intend it can also radically change the generated code and be a lot faster.

[@anon_y_mousse]

@@ABaumstumpf True, and I've never understood why C++ didn't adopt it also.

[@DART2WADER]

We need to bring what we started to the end)))

[@iosimion]

Hello. I would like to see not only a fix/modernize video, but also a memory profiler on the current version (at least for infiz). It would be interesting to see the results.

[@PedroOliveira-sl6nw]

I would love to see that (15:53) but don't just modernize. You can take the opportunity to teach about refactoring techniques and maybe even design patterns.

[@danielrhouck]

I’d love to see a video (or maybe it would take a series) on modernizing old code and this seems like a good option.

[@OnusofStrife]

Would love to see this compile and compare to the properly fixed version.

[@stephenhowe4107]

I have been programming since 1985 and I have *NEVER* had a problem with a bracing bug (or dangling else). So I make sure that if I have 1 statement => I leave out the braces.

[@brantregare]

Also coding since 1985, I never leave out braces because, braces make it really obvious where the scope begins and ends. No surprises. Even if you do not make such mistakes, it does not mean other team members, also do not. More importantly, do you really want to have to check and, possibly fix all code, not written by you in a team project. Reformatting tools like clang_format can help, they make it obvious, where there is a bracing error, or dangling else is. However, better to disambiguate, (never make the mistake in the first place,) from the beginning. You, "make sure," that means you double check, possibly triple check, to make it more difficult for others to read, maintain, and review, your code. My conclusion: you are not a team player. To each his/her own. At some point in time, your code will become legacy. Someone else will have to maintain it. Do you really want to make that person's life more difficult, because you are sublimely confident you do not make bracing errors at all, ever? If K&R, had simply disallowed non braced, If else blocks, this discussion would be moot, and a whole class of bugs would never have appeared. Yes, there was a compelling reason to allow it then, memory was expensive. Every byte mattered. Now, not so much. Just because you can, does not mean you should. To illustrate: using i and j as loop counters for a double nested loop, instead of meaningful names is misguided. Where you can, always use meaningful names. It is probably better to use ranges if you can. In generic algorithms, this is often, not possible. Your mileage may vary.

[@anon_y_mousse]

I'm with you on this one. I leave out braces unless there's more than one statement. I've not had any problems with teams, though, because I lay out the rules beforehand and anyone working on the project knows my style. Plus I try to weed out people that use K&R style bracing because it's obnoxious and error prone not being able to see leading braces. It's Allman style or you're off my team and that eliminates any excuse for missing braces.

[@stephenhowe4107]

@@brantregare : Well now, how far do you take this? You are not a team player if you do not use "this" before every member function or member data of a class (just so it is unambiguous as to the compiler) You are not a team player if you do not cast to void every call of a function that returns something You are not a team player if you do not make sure you use the scope operator for global data or functions (so the compiler always gets things right). You are not a team player if you do "using namespace std;" and not import from std what you use. The fact is using braces adds to the vertical clutter. That in itself, can cause programmers to make mistakes which is why K&R supported this. So it is arguably a mistake to insist that braces are put in even when not needed, I like my programs to be succinct with needless clutter removed. It would be interesting to find out your position on use of goto multiple returns from a function

[@stephenhowe4107]

@@anon_y_mousse: I would fit on your team easy. I don't like K & R brace style where the left brace is somewhere to the right and the matching brace is below and to the left. I like the braces to vertically line up on the left so it is easy on the eyeball, as God intended.

[@brantregare]

@@stephenhowe4107 Not using braces introduces a class of bugs that can be completely avoided. It takes active effort and awareness from everyone to prevent their accidental introduction. It also requires everyone to be aware of and ALWAYS check if a brace error was introduced. Disallowing them reduces the workload on the team. Legacy code maintainers will love you, a class of bugs not needed to be checked for. What you call needless clutter introduces bugs that can be very difficult to trace. Avoid. What you call needless another might call necessary. That is why we are having this discussion/exchange of opinion. My position on goto is: avoid if you can, use if you must, document in source with comments why and how you use it. With regard to multiple return paths: Often inverting conditional logic makes multiple return paths not necessary.

[@aypahyo]

I am super interested in watching you modernize this.

[@OzzFan1000]

As someone who is currently into retro-programming, I'm willing to bet you were using C With Classes, as I'm sure you know is the precursor to C++ which was standardized in 1998. If your university courses were in 97/98, it would make sense they were still teaching the old standard. To that end, the Standard Template Library was not a thing in most compilers, which means std::string was not available, nor were templates. Bools were not a primitive data type and were often typedef as ints. Most programmers from the time simply used int instead. That would explain some of your coding techniques. Though I do have a couple questions to help further my understanding of the language. Why do you prefer the stack and avoid pointers on the heap? Besides for memory leaks, is there another reason? Stack space seems to be quite limited in early DOS programs if you were using a small memory model. Also, why do you suggest stop passing and returning pointers? Is there another way to do this in C With Classes?

[@cppweekly]

I have gone back and looked at the old tools from that exact time frame and bool, std::string, std::vector, etc, all existed and were perfectly usable. The STL goes back to 94/96.

[@OzzFan1000]

@@cppweekly interesting. they don't exist in Microsoft Visual C++ 1.52, Symantec C++ 7.2, or it's successor Digital Mars C++ 8.52. Well, BOOL exists but only if you include I think.

[@wich1]

Please do a series of videos and treat this as an in production system that needs to keep running. Start by writing testers for it, fixing the bugs as you find them with the testers. Then continue on to modernizing the code while keeping tests running in some form of CI/CD pipeline. Pretty much every professional needs to deal with decades old code at some point in their career and pretty much every single one of them has no idea how to go about bringing legacy code up to date in an effective, efficient and least disruptive manner possible. All these people make the mistake of thinking that doing a green field implementation is the way to go only to find out as they go along that they were not aware of 80% of the requirements on the code as they were never written down anywhere, or at least anywhere that people can find.

[@CharlesHogg]

19:38 Really interesting pattern of forward declaring a bunch of functions, and _then_ including files. Seems like it would be pretty fragile!

[@atimholt]

I guess I don't know how worthwhile it actually would have been, but I was kind of hoping you'd fix the code up according to the then-current standard. Considering this was from before C++98, I guess that would have been kinda iffy.

[@cppweekly]

Stay tuned!

[@edwardng8899]

Yes, please complete the project and I would love to see the source code so I can also modernize it based on my knowledge.

[@MrAbrazildo]

I would like to see your game attempts with C++.

[@mattiaslaserskold137]

I would appriceate just sitting and modernising the code :)

[@rodneyjames1438]

Yes, do modernize this on video. That would be terrific. Would you, also please, give some context about what this program is trying to accomplish?

[@simonclark8290]

Go for it. See how much you can optimise it with modern techniques.

[@dangkhanhlinh476]

I'm proud of you Jason, you've improved a lot since college.

[@HansJMicheelsen]

Yes! Please modernize, fix and Update. And do use Clion

[@valizeth4073]

Where are your header guards!

[@greyfade]

Ohh, this hurts. I remember working with compilers this old and leaking memory like it's an unbounded resource. I want to fix this myself!

I think you were unfairly harsh on younger Jason. There are definitely plenty of things to improve on, but given that this was pre-std, the standard library of that time was tiny, little more that iostreams. There was no bool type, no string type, definitely no vector or stack. Very few compilers supported templates, and chances are that the one you had didn't. The memory leaks, the complete lack of const, the ill-chosen types are valid serious concerns even for the time period. It may have been a good idea to write your own mini-string class that did the things you needed.

[@cppweekly]

I did look into it - and I could have easily used string,vector etc! (See the upcoming episode about all this)

[@JonahFoley]

compiled tested and modernized 👍👍

[@TheSCSIBug]

Follow-up episode modernizing & writing tests? 💯👍

[@abtiwary]

Yes please! Please do consider doing follow up :)

[@winfr34k]

Yes of course I want to see you modernize that!

[@cat9078]

I would love to see this modernized

[@amingholizad]

It is good to see you were like noobs back then. It is encouraging for beginners. I would like to have this in a github repo and have a modernized version to check the diff.

[@cppweekly]

Fully history is here: github.com/cpp-best-practices/infiz

[@Antagon666]

Why would I want to make random functions nodiscard and noexcept ? Kind of bad language design if compiler doesn't optimize because of not including billions of extra keywords

[@sofasurfer3793]

you could use std::stack for Stack, no?

[@Caellyan]

std::vector is a stack (supports stack operations). std::stack is an adapter for containers (uses std::deque by default). But he said std::vector because it was available at the time, not sure whether std::stack was. But yeah, std::stack would be the best choice I think.

[@garyp.7501]

@@Caellyan std::stack was available as soon as the STL came out. It was in iteration 1.

[@paulojfonseca]

Yes a follow up video with modernization

[@magnusf2582]

#include do have a rarely used third form, #include PP_STRING, so you can do #define XXX #include XXX but you had #include foo.h and foo.h is not a PP_STRING so that is probably just a really lenient compiler. As for the wild indentation and #include "main.h", could it be that this was using some prepared skeleton so that ain't your code but something your professor provided?

[@ultradude5410]

This is actually kinda funny to me! Looks like there’s tokenizing going on. I’m currently in university, and just wrote a Java lexer! (Unfortunately, in Java)

[@bryce.ferenczi]

The sad thing is that this still looks like a lot of student code today in 2023.... Some university lecturers still teach this style of code....

[@hinzster]

The use of void pointers seems to have been all the rage in 1998, be it in C++ or just plain C. If you look at C code from the same vintage (like eg. openssl) it'll be riddled with void pointers, because checking types is apparently for cowards. And in openssl it actually scares me. And I most certainly would want to see you modernize and best-practice this code, I can't remember a single of your videos where I didn't learn something, and this would be a perfect learning opportunity.

[@cppweekly]

Stay tuned!

[@MichaelSchulzDaFritzmark]

Please do the follow-up! Would be cool if you use your cop default project.

[@kurtmayer2041]

hell yeah i'd love to see that modernization

[@multiHappyHacker]

pssst hey kid, wanna know how to irreparably screw up a visual studio project? Just move all the standard library includes below the project includes. It will never build again because visual studio will be broken and won't detect the project includes being included anymore. This was possible back in like 2015 too, but apparently they never fixed it.

[@OlaInTheClouds]

This is so funny, I feel you. around 12 minutes "I was learning java at the same time" cringe... Thanks for sharing this horror!

[@szaszm_]

This reminds me of a simple socks proxy server I wrote in C as a chosen 1st year university homework. It has some leaks and occasionally hangs. I should rewrite it in modern C++, it's only about 300 lines.

[@benjaminshinar9509]

Is the project supposed to read a string math expression and return a result? like "1 + 2 / (3*-4)"? it feels like that but wasn't clear. could you show the driver code or tests if you do a similar video?

[@cppweekly]

I have a follow-up video that will be published soon! (make sure you're subscribed!) But at the time I had no tests at all and no expected results, but yes, it's supposed to evaluate any infix math expression. Theoretically.

[@benjaminshinar9509]

@@cppweekly watching it now!

[@milasudril]

I would like to see clang-tidy suggested fixes

[@redrj]

You should run sanitizer, see all the errors, then modernize it, then run the sanitizer again 😅

[@davealias]

Lol yes, please follow up and fix this hilarious code!

[@yogthemuskrat]

You can do it, Jason!