Skip to content

Conversation

@jjuhl
Copy link
Contributor

@jjuhl jjuhl commented Jan 14, 2026

Hi Texus

Here's another mixed bag of fixes/cleanups for you.
As always, let me know if you need me to make any changes, otherwise feel free to merge as-is :)

Kind regards,
Jesper Juhl

jjuhl added 7 commits January 14, 2026 16:30
More readable and avoids creating temporary objects for comparison.
When the default constructor does the same as initializing a member
with {} in the member initializer list, that explicit initialization
is redundant.

When a menber is initialized using an in-class initializer and then is
also explicitly initialized in the member initialization list of the
constructor, the latter is redundant and does nothing.
jjuhl added 5 commits January 14, 2026 20:25
Just output a newline when that is all that is needed - no reason to
include the implicit std::flush that std::endl does in these cases,
that's just extra work for no gain.
C-style casts are dangerous since they can perform a number of
different casts (and in the end can even fall back to
reinterpret_cast<const_cast<>> which is very rarely what one wants).
In addition, C-style casts are super hard to search for with tools
like 'grep' etc.

Always prefer C++ casts when possible.
To avoid static analyzer tools complaining about some cases not being
covered.
…n and implementation

It's confusing when parameters are named differently between header
and implementation files.
This fixes up a few cases of inconsistent naming.
@texus
Copy link
Owner

texus commented Jan 15, 2026

Thanks again for your work.

I cherry-picked those commits because I didn't like the "Remove some instances of redundant member initialization" commit. I like having empty initializers in copy and move constructors to indicate that I'm doing something special. Otherwise it could raise a question whether I maybe forgot to add the member to the copy constructor (which is a mistake that has happened a few times already when adding new members). It also clearly shows for which members there is an exception, for when you wonder why the default copy constructor couldn't be used.

For the initializer list of normal constructors I would be ok with removing the redundant code, because not all members are listed there anyway (because most are being initialized at the declaration in the header file instead of in the initializer list).

Based on you making "Container::saveWidgetsToFile" const, I also made "BackendGui::saveWidgetsToFile" a const function. The BackendGui has almost the same interface as Container, but it doesn't inherit from it because a Gui isn't a widget and it would also inherit functionality that wouldn't make sense.

@texus texus closed this Jan 15, 2026
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.

2 participants