Skip to content

pass by reference or by value when necessary#228

Merged
atsju merged 5 commits intogithubdoe:masterfrom
atsju:JST/clazyArgs
Aug 20, 2025
Merged

pass by reference or by value when necessary#228
atsju merged 5 commits intogithubdoe:masterfrom
atsju:JST/clazyArgs

Conversation

@atsju
Copy link
Collaborator

@atsju atsju commented Aug 15, 2025

I have used clazy to automatically use refs and values where it should.
QPoint is misleading but it's indeed a simple type to pass by value.
Notice there is no big risk changing type a to const type &a and reverse.

Those changes should bring a marginal efficiency gain. Nothing really measurable.

@atsju atsju requested review from githubdoe and gr5 August 15, 2025 10:33
}

void ContourPlot::selected(const QPointF& pos){
void ContourPlot::selected(QPointF pos){
Copy link
Collaborator

Choose a reason for hiding this comment

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

I get that you are adding tons of "const &" and that makes it so it uses the incoming variable as is but promises not to modify it.

But why are you removing the const & in a few cases such as this one?

Copy link
Collaborator Author

@atsju atsju Aug 17, 2025

Choose a reason for hiding this comment

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

Basically passing large classes by value makes the compiler do a copy of the variable on the stack. That's why you pass by reference with & (I'm a C expert so I see this more or less like pass by pointer). Adding const tells compiler and reviewer that the variable will not be modified inside the function.

You don't want to pass an int by pointer or reference because it adds unnecessary operation to make the pointer or reference. Small variables are already in registers and don't need copy on stack.

QPointF is a class but in fact it's really just 2 values. So it's small enough to prefer passing by value.
Now I understand this one (QPointF) is really misleading and I don't think it changes anything to the code. I would almost lean to let it like it was with pass by reference. It's just a little more work as those chances were automated.

Copy link
Collaborator

@gr5 gr5 left a comment

Choose a reason for hiding this comment

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

These changes look extremely safe and a good improvement.

@atsju atsju merged commit 15632cb into githubdoe:master Aug 20, 2025
14 checks passed
@atsju atsju deleted the JST/clazyArgs branch December 23, 2025 14:03
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

Comments