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

Octree Iterator begin/end method and added tests #2174

Merged
merged 2 commits into from
Jan 27, 2018

Conversation

frozar
Copy link
Contributor

@frozar frozar commented Jan 3, 2018

I propose to delete begin()/end() iterator method definitions in class OctreePointCloud and OctreePointCloudAdjacency. As these classes derived from OctreeBase, which defines all iterator methods, it is not necessary and confusing to redefine them.

I also add some test over the OctreePointCloudAdjacency to check the basic iterator behavior (construction, comparison).

@frozar frozar force-pushed the iterator_test_cleaning branch 4 times, most recently from 9084d3d to b0bc08d Compare January 9, 2018 13:10
@SergioRAgostinho SergioRAgostinho added the needs: code review Specify why not closed/merged yet label Jan 9, 2018
Copy link
Member

@SergioRAgostinho SergioRAgostinho left a comment

Choose a reason for hiding this comment

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

This looks totally ok. I just feel it misses that test you added more recently actually iterating through the octree, counting branch and leaf nodes, etc... Basically making sure it works. I'm ok without it nevertheless.

@frozar
Copy link
Contributor Author

frozar commented Jan 15, 2018

We this implementation, every test from test/octree/test_octree.cpp and test/octree/test_octree_iterator.cpp pass without any problem.

In test/octree/test_octree.cpp you have a bench of existent test which check the number of branch and leaf nodes and so on over an octree, so for me it seems OK.

Nevertheless I plan to add more tests over every kind of iterator in PR #1983

@frozar frozar force-pushed the iterator_test_cleaning branch from b0bc08d to d352b15 Compare January 23, 2018 17:04
@SergioRAgostinho SergioRAgostinho added this to the pcl-1.9.0 milestone Jan 25, 2018
@taketwo taketwo added status: ready to merge and removed needs: code review Specify why not closed/merged yet labels Jan 27, 2018
@taketwo taketwo merged commit 824541a into PointCloudLibrary:master Jan 27, 2018
@frozar frozar deleted the iterator_test_cleaning branch January 27, 2018 10:56
@SergioRAgostinho SergioRAgostinho changed the title [TEST] OCTREE Iterator begin/end method and add test Octree Iterator begin/end method and added tests Aug 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants