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

Getters in SampleConsensus should have const qualifiers #2959

Closed
PPokorski opened this issue Apr 1, 2019 · 3 comments · Fixed by #2970
Closed

Getters in SampleConsensus should have const qualifiers #2959

PPokorski opened this issue Apr 1, 2019 · 3 comments · Fixed by #2970
Labels
good first issue Skills/areas of expertise needed to tackle the issue module: sample_consensus

Comments

@PPokorski
Copy link
Contributor

Your Environment

  • PCL Version: master branch

Context

Some methods in SampleConsensus class.

inline double
getDistanceThreshold () { return (threshold_); }

inline int
getMaxIterations () { return (max_iterations_); }

inline double
getProbability () { return (probability_); }

Expected Behavior

Those methods should be marked as const.

Current Behavior

const qualifier is not present.

Code to Reproduce

I think examples above are enough :)

Possible Solution

Add const qualifier.

@PPokorski PPokorski changed the title Getters should have const qualifiers Getters in SampleConsensus should have const qualifiers Apr 1, 2019
@SergioRAgostinho
Copy link
Member

Agreed. Feel free to open a PR with the changes.

@PPokorski
Copy link
Contributor Author

Thank you. One more thing from MaximumLikelihoodSampleConsensus: there are some methods like those:

double
computeMedianAbsoluteDeviation (const PointCloudConstPtr &cloud,
const boost::shared_ptr <std::vector<int> > &indices,
double sigma);

void
getMinMax (const PointCloudConstPtr &cloud,
const boost::shared_ptr <std::vector<int> > &indices,
Eigen::Vector4f &min_p,
Eigen::Vector4f &max_p);

void
computeMedian (const PointCloudConstPtr &cloud,
const boost::shared_ptr <std::vector<int> > &indices,
Eigen::Vector4f &median);

which, I think, could also be const. What do you think?

@taketwo
Copy link
Member

taketwo commented Apr 5, 2019

Sure, these are good candidates to be marked const as well.

@taketwo taketwo added the good first issue Skills/areas of expertise needed to tackle the issue label Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Skills/areas of expertise needed to tackle the issue module: sample_consensus
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants