Skip to content

Enhance Efficiency, Readability, and Performance in Various Cppcheck Modules#6650

Closed
merttozer wants to merge 1 commit intocppcheck-opensource:mainfrom
merttozer:improve-cppcheck
Closed

Enhance Efficiency, Readability, and Performance in Various Cppcheck Modules#6650
merttozer wants to merge 1 commit intocppcheck-opensource:mainfrom
merttozer:improve-cppcheck

Conversation

@merttozer
Copy link
Copy Markdown

This pull request includes a series of improvements aimed at enhancing the efficiency, readability, and performance of several key modules in the Cppcheck project. The changes made are detailed as follows:

  • executor.cpp:

    • Used initializer lists in the constructor
    • Simplified assertion and used std::lock_guard for mutex
  • singleexecutor.cpp and singleexecutor.h:

    • Simplified constructor initialization
    • Used range-based for loops and std::accumulate with lambda for better readability and efficiency
  • aboutdialog.cpp and aboutdialog.h:

    • Replaced raw pointers with std::unique_ptr
    • Simplified string formatting and used modern C++ features
    • Improved memory management and added const correctness
  • applicationdialog.cpp and applicationdialog.h:

    • Replaced raw pointers with std::unique_ptr
    • Improved memory management and added const correctness

- executor.cpp:
  - Used initializer lists in the constructor
  - Simplified assertion and used std::lock_guard for mutex

- singleexecutor.cpp and singleexecutor.h:
  - Simplified constructor initialization
  - Used range-based for loops and std::accumulate with lambda for better readability and efficiency

- aboutdialog.cpp and aboutdialog.h:
  - Replaced raw pointers with std::unique_ptr
  - Simplified string formatting and used modern C++ features
  - Improved memory management and added const correctness

- applicationdialog.cpp and applicationdialog.h:
  - Replaced raw pointers with std::unique_ptr
  - Improved memory management and added const correctness
@firewave
Copy link
Copy Markdown
Collaborator

Thanks for your contribution.

The PR appears to be incomplete though as some changes only have been partially applied and the description does not match the actual changes.

Also for various reasons it would be better to group the various changes into separate PRs.

Also if you claim performance improvement please provide some actual proof.

Comment thread gui/applicationdialog.cpp
QWidget *parent) :
QDialog(parent),
mUI(new Ui::ApplicationDialog),
mUI(std::make_unique<Ui::ApplicationDialog>()),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

std::make_unique is not available until C++14 and the code targets C++11.

Comment thread cli/singleexecutor.cpp
std::size_t processedsize = 0;
unsigned int c = 0;

for (std::list<FileWithDetails>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We have not changed such loops yet since they are in a mutable scope and you cannot fully enforce the intent of constness in a range loop until C++17 which introduces std::as_const (I have been trying to add a helper for that).

That seems pedantic but since changing the code does not actually change anything it doesn't matter which style you use.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Coincidentally I finally ran into some code which highlights the issue - see #6653.

@firewave
Copy link
Copy Markdown
Collaborator

No reply in over a month. Closing.

@firewave firewave closed this Aug 31, 2024
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