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

Use recursionDepth+1 instead of ++recursionDepth #139

Closed
samreid opened this issue Mar 27, 2024 · 1 comment
Closed

Use recursionDepth+1 instead of ++recursionDepth #139

samreid opened this issue Mar 27, 2024 · 1 comment
Assignees

Comments

@samreid
Copy link
Member

samreid commented Mar 27, 2024

During code review #103, we observed this code:

// Did we overshoot the curve? If so, call this method recursively.
if ( coilSegmentPosition < 0.0 ) {
this.switchCurves( coilSegmentPosition, ++recursionDepth );

This code incorrectly increments the recursionDepth variable. Someone debugging the code and looking at that value after the recursive call completed would see an incorrect value for recursionDepth.

This can be remedied by changing it to recursionDepth+1 in both cases.

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 28, 2024

Good point. I replaced ++recursionDepth with recursionDepth + 1 in the above commit.

FYI, recursionDepth was not present in the Java implementation. It was added as a safeguard, and its only purpose is to prevent infinite recursion via this assertion:

assert && assert( recursionDepth < this.coilSegments.length, `infinite loop? recursionDepth=${recursionDepth}` );

Closing.

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

No branches or pull requests

2 participants