Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further cleanup #438

Merged
merged 4 commits into from
Oct 5, 2022
Merged

Further cleanup #438

merged 4 commits into from
Oct 5, 2022

Conversation

stefanos82
Copy link

No description provided.

When you want to pass a `std::string` to `std::string_view`,
prefer to do such operation during object initialization via `{}` so you
can avoid to use the copy constructor, which can be expensive under
certain situations.
I haven't touched `include/robin_hood.h` as I don't know whether it's a
third-party header file that could get rewritten at a later time of a
possible future release by the author(s) of it.
Copy link
Owner

@aristocratos aristocratos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍🏼

@aristocratos aristocratos merged commit d58a17a into aristocratos:main Oct 5, 2022
@imwints
Copy link
Contributor

imwints commented Jul 16, 2023

@stefanos82
@aristocratos
But why would we do this? Why allow variables, which are not changing, to be mutated? You should keep all those bools as const if they're not reassigned in the function bodies.

@stefanos82
Copy link
Author

But why would we do this? Why allow variables, which are not changing, to be mutated? You should keep all those bools as const if they're not reassigned in the function bodies.

No need to, because false and true are prvalues of type bool; therefore the compiler will optimize their const-ness out.

If we are dealing with actual objects, such as classes then yes, it would have played a major role to retain our const type qualifier, even though nowadays with move semantics, you don't always need to deal with const someObject&, thus you can reduce memory usage just by move a lot of data around with move semantics.

Anyway, long story short, the optimizer was already discarding the const type qualifier from bool variables and decided to clean it a bit; that's it, really.

@imwints
Copy link
Contributor

imwints commented Jul 17, 2023

I hope this PR was not done for any performance gains. That's not what this is about. Putting const on a trivial type has no speed up effect at all, this should be clear.

This is about readability and maintainability.

https://stackoverflow.com/questions/22285773/use-const-wherever-possible-in-c/22285981#22285981

If you find ordinary type safety helps you get systems correct (it does; especially in large systems), you'll find const correctness helps also.

You should use const when you want to be sure not to change variable accidentally or intentionally. Some constants (globals and class static, strings & integers, but not variables with nontrivial constructor) can be placed in read-only parts of the executable, therefore result in segmentation fault if you try to write to it.

and

[...] You should be using const whereever it is possible and makes sense.

From here:

const correctness can't improve performance because const_cast and mutable are in the language, and allow code to conformingly break the rules. [...]

That all said, const correctness is a good thing with respect to maintainability. [...]

Don't use const for performance reasons. Use it for maintainability reasons.

Something more legitimate might be IsoCPP:

What is “const correctness”?
A good thing. It means using the keyword const to prevent const objects from getting mutated. [...]

Should I try to get things const correct “sooner” or “later”?
At the very, very, very beginning.
Back-patching const correctness results in a snowball effect: every const you add “over here” requires four more to be added “over there.”
Add const early and often.

and another opinion right behind:

Doom's code is fairly rigid, although not rigid enough in my opinion with respect to const. Const serves several purposes which I believe too many programmers ignore. My rule is "everything should always be const unless it can't be". I wish all variables in C++ were const by default. Doom almost always sticks to a "no in-out" parameter policy; meaning all parameters to a function are either input or output never both. This makes it much easier to understand what's happening with a variable when you pass it to a function.

https://github.com/cpp-best-practices/cppbestpractices/blob/master/04-Considering_Safety.md:

const tells the compiler that a variable or method is immutable. This helps the compiler optimize the code and helps the developer know if a function has a side effect.

It is for the developer to make fewer mistakes and having a better time reading the code and understanding it.

@stefanos82
Copy link
Author

Since you are so pedantic and most probably have years of professional experience, by all means feel free to PR anytime and help this project with your expertise.

Cheers 👍

imwints added a commit to imwints/btop that referenced this pull request Jul 17, 2023
Revert the removal of const qualifiers on bool paramteres and slap const
on all variables that allow it to avoid accidental mutations and improve
code readability
imwints added a commit to imwints/btop that referenced this pull request Jul 17, 2023
Revert the removal of const qualifiers on bool paramteres and slap const
on all variables that allow it to avoid accidental mutations and improve
code readability
imwints added a commit to imwints/btop that referenced this pull request Jul 17, 2023
Revert the removal of const qualifiers on bool paramteres and slap const
on all variables that allow it to avoid accidental mutations and improve
code readability
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants