C++ Weekly - Ep 402 - Reviewing My 25 Year Old C++ Code (IT'S BAD!)
Vložit
- čas přidán 12. 11. 2023
- ☟☟ Awesome T-Shirts! Sponsors! Books! ☟☟
Upcoming Workshop: Understanding Object Lifetime, C++ On Sea, July 2, 2024
► cpponsea.uk/2024/sessions/und...
Upcoming Workshop: C++ Best Practices, NDC TechTown, Sept 9-10, 2024
► ndctechtown.com/workshops/c-b...
Create anything, find anything, fix anything, and navigate to anything with C++ and CLion.
A cross-platform IDE for C and C++ with:
- A smart C and C++ editor to navigate and maintain your code base productively.
- Code analysis with quick-fixes to identify and fix bugs and style inconsistencies.
- An integrated debugger - along with other essential tools from the ecosystem - available straight out of the box.
Learn more: jb.gg/clion_ide
Use `CppWeeklyCLion` to get 25% OFF when you purchase a new CLion subscription or renew your existing one.
Episode Details: github.com/lefticus/cpp_weekl...
T-SHIRTS AVAILABLE!
► The best C++ T-Shirts anywhere! my-store-d16a2f.creator-sprin...
WANT MORE JASON?
► My Training Classes: emptycrate.com/training.html
► Follow me on twitter: / lefticus
SUPPORT THE CHANNEL
► Patreon: / lefticus
► Github Sponsors: github.com/sponsors/lefticus
► Paypal Donation: www.paypal.com/donate/?hosted...
GET INVOLVED
► Video Idea List: github.com/lefticus/cpp_weekl...
JASON'S BOOKS
► C++23 Best Practices
Leanpub Ebook: leanpub.com/cpp23_best_practi...
► C++ Best Practices
Amazon Paperback: amzn.to/3wpAU3Z
Leanpub Ebook: leanpub.com/cppbestpractices
JASON'S PUZZLE BOOKS
► Object Lifetime Puzzlers Book 1
Amazon Paperback: amzn.to/3g6Ervj
Leanpub Ebook: leanpub.com/objectlifetimepuz...
► Object Lifetime Puzzlers Book 2
Amazon Paperback: amzn.to/3whdUDU
Leanpub Ebook: leanpub.com/objectlifetimepuz...
► Object Lifetime Puzzlers Book 3
Leanpub Ebook: leanpub.com/objectlifetimepuz...
► Copy and Reference Puzzlers Book 1
Amazon Paperback: amzn.to/3g7ZVb9
Leanpub Ebook: leanpub.com/copyandreferencep...
► Copy and Reference Puzzlers Book 2
Amazon Paperback: amzn.to/3X1LOIx
Leanpub Ebook: leanpub.com/copyandreferencep...
► Copy and Reference Puzzlers Book 3
Leanpub Ebook: leanpub.com/copyandreferencep...
► OpCode Puzzlers Book 1
Amazon Paperback: amzn.to/3KCNJg6
Leanpub Ebook: leanpub.com/opcodepuzzlers_book1
RECOMMENDED BOOKS
► Bjarne Stroustrup's A Tour of C++ (now with C++20/23!): amzn.to/3X4Wypr
AWESOME PROJECTS
► The C++ Starter Project - Gets you started with Best Practices Quickly - github.com/cpp-best-practices...
► C++ Best Practices Forkable Coding Standards - github.com/cpp-best-practices...
O'Reilly VIDEOS
► Inheritance and Polymorphism in C++ - www.oreilly.com/library/view/...
► Learning C++ Best Practices - www.oreilly.com/library/view/... - Věda a technologie
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!
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.
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.
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 )
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.
I agree. We all would learn much more.
Stay tuned!
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.
That’s my second C++ weekly video and I definitely want to see a follow up on that 😂 Fix it, modernize it, benchmark it!
Hello, please fix it and modernize it fully. This is so instructive !
More please. This made me feel good about my own (not nearly as bad) C++.
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)
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!
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++.
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.
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!
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.
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!)
Please, oh, please yes, do it! Fix and modernize that... stuff.
This was hella fun!
Great. Love to see that relic modernised, compiled and running.
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.
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.
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.
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.
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.
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
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.
Absolutely. Sometimes modernizing the tool-chain is NOT allowed.
Same, I would like to get a video with code fixed perhaps to the 98 standard. And a second video that modernizes it.
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.
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
Also visible there, string is allocated with the array form of new, but the destructor releases string with non-array delete.
@@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,
@@stephenhowe4107 Indeed to both! This comes up in the next episode about this topic.
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.
Jason, I was so entertained by your reactions! Sadly, I have the same response to too much code I review to this day 😂
It's always fun to go down a memory lane. Great video. Other comments have said it enough. We definitely need a follow up.
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.
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.
Yes that would be interesting as well.
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!
Do it. MAKE IT WORK.
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`.
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.
I'd definitely watch the follow-up episode; it could be yet another learning experience!
15:00 Yes, modernize it in another episode! But what about modernize it by iteration throughout all standards from 98 to 23?
Brilliant.. your pain was palpable!
Yes we definitely want to see how you modernize, test and run this code
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.
I'd love to see the modernized and sanitized version!
Oh my goodness. This was an amazing video to watch. I'd love to see this code getting modernized!
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
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.
Hi Jason, would be really nice to see the code completly refactored
Yes please! Please do consider doing follow up :)
We need to bring what we started to the end)))
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.
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++.
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.
Please do make a follow-up episode in which you fix/modernized this code.
I would appriceate just sitting and modernising the code :)
Nice. Would like to see it compile with old and modern compilers.
Yes, please do a follow up and fix/modernize the code. Thank you.
Yes a follow up video with modernization
Best quote after seeing all the allocations and pointers used: "Oh, I was learning Java at the same time". 😂🤣
Yes of course I want to see you modernize that!
Would love to see this compile and compare to the properly fixed version.
I am super interested in watching you modernize this.
compiled tested and modernized 👍👍
Go for it. See how much you can optimise it with modern techniques.
Nice video! Would be educative to watch you modernize your code.
Yeah, please fix this and get this code compiling, it would be so interesting!
I would LOVE to see you modernise this code. Just’ve ended implementing my own infix notation(shunting yard algorithm) parser and RPN evaluator.
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.
I would like to see you refactor this code and compile it!! Thanks and I'm glad you make this video
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
please modernize the code and make it work, would also love to see you coding stuff in c++, while explaining your thought process ❤️❤️
19:38 Really interesting pattern of forward declaring a bunch of functions, and _then_ including files. Seems like it would be pretty fragile!
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.
Yes, please complete the project and I would love to see the source code so I can also modernize it based on my knowledge.
intro.h includes main.h but main.h also includes intro.h
You can do it, Jason!
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.
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.
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.
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.
@@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
@@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.
@@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.
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.
I would love to see this modernized
Yes! Please modernize, fix and Update. And do use Clion
'void main'? Surprised you didn't call that out! Was that ever correct in C++?
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.
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.
Please do the follow-up!
Would be cool if you use your cop default project.
hell yeah i'd love to see that modernization
Did they have clang-formatter in 1998? Because without that thing I still might be inconsistent with tabbing, braces, spaces and such
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....
This is so funny, I feel you. around 12 minutes "I was learning java at the same time" cringe... Thanks for sharing this horror!
I literally can see my own old code right now 😅
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!
Definitely want to see it modernized!!!!!!!
I would like to see clang-tidy suggested fixes
Follow-up episode modernizing & writing tests? 💯👍
I would like to see your game attempts with C++.
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.
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.
@@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.
@@ABaumstumpf True, and I've never understood why C++ didn't adopt it also.
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.
Yes make it running and tested.
its so good plz do it modernized and compilebale and tetable tnx for your amazing videos so fun
Where are your header guards!
I'm proud of you Jason, you've improved a lot since college.
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.
Fully history is here: github.com/cpp-best-practices/infiz
Why not use stack adapter?
Yes please fix it, modernize it. It would be amazing to see how modern features in the language will help simplifying the code and making it efficient and leak free.
Lol yes, please follow up and fix this hilarious code!
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?
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.
@@cppweekly watching it now!
Yes. Cleaning video please
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)
you could use std::stack for Stack, no?
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.
@@Caellyan std::stack was available as soon as the STL came out. It was in iteration 1.
#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?