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

Replace postfix increment and decrement operators with their prefix equivalents #385

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

krishnakumarg1984
Copy link
Collaborator

@krishnakumarg1984 krishnakumarg1984 commented Jun 23, 2023

There is no reason whatsoever to use the postfix versions of the increment and decrement operator.

In postfix incrementing, first, a copy of the operand is made. Then the operand (not the copy) is incremented. Finally, the copy (not the original) is evaluated. Then the temporary copy is discarded. The postfix version takes a lot more steps, and thus may not be as performant as the prefix version.

In practice, any non-toy compiler will optimise away these steps and the postfix version is likely to have the same performance as the prefix versions for fundamental data types and even for compound types.

However, for user-defined data types where these operators may be overloaded, the postfix version is likely to be slower.

References

There should be no reason whatsoever to use these postfix operators, and we should strive to break this old habit asap.

@krishnakumarg1984 krishnakumarg1984 self-assigned this Jun 23, 2023
@krishnakumarg1984 krishnakumarg1984 changed the title replace postfix increment and decrement operators with their prefix equivalents Replace postfix increment and decrement operators with their prefix equivalents Jun 23, 2023
@krishnakumarg1984 krishnakumarg1984 force-pushed the kg/replace_postfix_operators_with_prefix branch from d0e2bba to 9486d9e Compare June 23, 2023 14:01
@SJaffa
Copy link
Contributor

SJaffa commented Jul 3, 2023

I've always seen loops taught using the postfix operator so this might be a case where it is best to stick with the convention so as not to confuse less experienced programmers. Is this likely to give a significant performance increase? If not I'd favour readability for the researchers.

@krishnakumarg1984
Copy link
Collaborator Author

krishnakumarg1984 commented Jul 3, 2023

@SJaffa

This blog-post is an eye-opening read on pre-increment vs post-increment for loops.

Guideline: Prefer preincrement. Only use postincrement if you’re going to use the original value.


If not I'd favour readability for the researchers.

Did you mean familiarity? They are both equally readable, no?

I've always seen loops taught using the postfix operator

This abhorrent practice needs to stop sooner than later :) Have a look at this answer or this one.

Is this likely to give a significant performance increase?

Yes. For user-defined types (not in this PR though).

The post-incrementation i++ needs to create a temporary variable to store the original value of i, then performs the incrementation and returns the temporary variable. The pre-incrementation ++i doesn't create a temporary variable.


In postfix incrementing, first, a copy of the operand is made. Then the operand (not the copy) is incremented. Finally, the copy (not the original) is evaluated. Then the temporary copy is discarded. The postfix version takes a lot more steps, and thus may not be as performant as the prefix version.

@SJaffa
Copy link
Contributor

SJaffa commented Jul 4, 2023

If not I'd favour readability for the researchers.

Did you mean familiarity? They are both equally readable, no?

I mean readability of the code as a whole. While i++ and ++i are equally quick to read, having even small bits of unfamiliar syntax makes the code significantly harder to read and digest. I learned this from the Programmer's Brain which we are reading in ARC book club, I can lend it to you if you like, really good read!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants