[sample_consensus] `Sac::getInliers` should return the results as a const reference instead of to a parameter

[SunBlack]

Just found this

pcl/sample_consensus/include/pcl/sample_consensus/sac.h

Lines 309 to 310 in 20464ce

	inline void
	getInliers (Indices &inliers) const { inliers = inliers_; }

Compared to this, the GPU module is already fine (apart from the slightly different type between both modules)

pcl/cuda/sample_consensus/include/pcl/cuda/sample_consensus/sac.h

Lines 154 to 155 in 20464ce

	inline IndicesPtr
	getInliers () { return inliers_; }

Note: Sames goes for getModelCoefficients

[mvieth]

@SunBlack Hi, could you elaborate on this? Why should the functions return a const reference?

[SunBlack]

With the current method, a copy is forcibly created (inliers_ is copied to inliers). However, if inliers_ is returned as a const reference, you can work directly on inliers_ unless you want to modify it. This leaves it up to the user of the method to decide whether a copy is necessary or not.

[mvieth]

Ah okay. that makes sense. Feel free to create a pull request to add the new functions, if you like. I can also do that sometime, but can't promise when. I would suggest to only add the additional functions but not deprecate or remove the existing getInliers/getModelCoefficients. It is good to give users the option to avoid the copy, but I think that is not enough reason to force users to switch. However, we could replace all usages within PCL.

[SunBlack]

This is not a priority for us. I just noticed that as we are currently removing the last remnants of the PCL from our code (as usage has fallen sharply in recent years), I haven't touched my open PRs and issues for a long time (which may already be obsolete).

[jose-n9]

Is this still available to create a pull request for getInliers?

[mvieth]

@jose-n9 Hi! Yes, it is still available. If you would like to create a pull request, please also add new functions for getModel and getModelCoefficients.

[jose-n9]

@mvieth I created a pull request, thank you

[mvieth]

@jose-n9 I requested some changes on the pull request, please address them. Thanks.