Skip to content

Add the ability to merge IdLists, and use to banish more non-range-for. #461

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

Closed
wants to merge 1 commit into from

Conversation

rpavlik
Copy link
Contributor

@rpavlik rpavlik commented Aug 20, 2019

Eliminates a class of reasons for not using range-for: Adding
during iteration. Instead, just add to a new IdList that you
merge in after iteration is complete.

In general I didn't convert the new types of solids to use this, just because I'm still new at using them so I couldn't tell for sure if it worked right. But, all tests pass.

Eliminates a class of reasons for not using range-for: Adding
during iteration. Instead, just add to a new IdList that you
merge in after iteration is complete.
++outIter;
}

/// @todo Look to see if we already have something with the same handle value.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not really comfortable merging this without an assert here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I just realized this messes up a bunch of things if you actually try using a lathe. I'm dropping it, sorry.

Sorry, something went wrong.

@rpavlik rpavlik closed this Aug 20, 2019
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.

None yet

2 participants