Access control modifier change in VoxelGrid

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Access control modifier change in VoxelGrid

Brian Okorn
PCL Developers,

I was inheriting from VoxelGrid and noticed that VoxelGrid.h is changing the access modifier from protected to private for several variables (filter_name_, input_, ect.) when using is used without a access modifier.  This has cause problems accessing those variables from a child class.  Is there a reason for doing this?  If not maintaining the protected access modifier makes it easier to inherit from (putting "protected:" before the using statements in the VoxelGrid class definition).

v/r,
Brian Okorn

_______________________________________________
PCL-developers mailing list
[hidden email]
http://pointclouds.org/mailman/listinfo/pcl-developers
http://pointclouds.org
Reply | Threaded
Open this post in threaded view
|

Re: Access control modifier change in VoxelGrid

Radu B. Rusu
Administrator
Brian,

That's an oversight on our side. Can you submit a patch that fixes this and we'll incorporate it immediately? Thanks a lot!

Cheers,
Radu.
--
Point Cloud Library (PCL) - http://pointclouds.org

On 09/20/2011 10:13 AM, Brian Okorn wrote:

> PCL Developers,
>
> I was inheriting from VoxelGrid and noticed that VoxelGrid.h is changing the access modifier from protected to private
> for several variables (filter_name_, input_, ect.) when using is used without a access modifier.  This has cause
> problems accessing those variables from a child class.  Is there a reason for doing this?  If not maintaining the
> protected access modifier makes it easier to inherit from (putting "protected:" before the using statements in the
> VoxelGrid class definition).
>
> v/r,
> Brian Okorn
>
>
> _______________________________________________
> PCL-developers mailing list
> [hidden email]
> http://pointclouds.org/mailman/listinfo/pcl-developers
> http://pointclouds.org
_______________________________________________
PCL-developers mailing list
[hidden email]
http://pointclouds.org/mailman/listinfo/pcl-developers
http://pointclouds.org
Reply | Threaded
Open this post in threaded view
|

Re: Access control modifier change in VoxelGrid

Julian Löchner
In reply to this post by Brian Okorn
This is something I really had a problem with when I started to use PCL. The change of the access level (private, protected, public) happens in many classes of PCL. For example:
In pcl/features/feature.h, class Feature, the access level of indices_ and input_ is changed from protected to public.
In pcl/filters/bilateral.h, class BilateralFilter, indices_ and input_ are changed to private.
In pcl/keypoints/keypoint.h, class Keypoint, indices_ and input_ are again changed to public.

These are only a few examples where this is the case. For a filter class (change to private) this means that I can't derive a subclass. In the tutorial "Writing a new PCL class" it is suggested to use "using" without noting that this will change the access level if we do not define the right one.

Cheers, Julian
Reply | Threaded
Open this post in threaded view
|

Re: Access control modifier change in VoxelGrid

Radu B. Rusu
Administrator
Julian,

There might be oversights. It's best we fix them all carefully - "given enough eyeballs, all bugs are shallow"? :)

Can you fire up individual issues on http://dev.pointclouds.org, and we can follow up there? Thanks so much!

Cheers,
Radu.
--
http://pointclouds.org

On 12/22/2011 02:06 AM, Julian Löchner wrote:

> This is something I really had a problem with when I started to use PCL. The
> change of the access level (private, protected, public) happens in many
> classes of PCL. For example:
> In pcl/features/feature.h, class Feature, the access level of indices_ and
> input_ is changed from protected to public.
> In pcl/filters/bilateral.h, class BilateralFilter, indices_ and input_ are
> changed to private.
> In pcl/keypoints/keypoint.h, class Keypoint, indices_ and input_ are again
> changed to public.
>
> These are only a few examples where this is the case. For a filter class
> (change to private) this means that I can't derive a subclass. In the
> tutorial "Writing a new PCL class" it is suggested to use "using" without
> noting that this will change the access level if we do not define the right
> one.
>
> Cheers, Julian
>
> --
> View this message in context: http://www.pcl-developers.org/Access-control-modifier-change-in-VoxelGrid-tp4823298p5094195.html
> Sent from the Point Cloud Library (PCL) Developers mailing list archive at Nabble.com.
> _______________________________________________
> PCL-developers mailing list
> [hidden email]
> http://pointclouds.org/mailman/listinfo/pcl-developers
> http://pointclouds.org
_______________________________________________
PCL-developers mailing list
[hidden email]
http://pointclouds.org/mailman/listinfo/pcl-developers
http://pointclouds.org
Reply | Threaded
Open this post in threaded view
|

Re: Access control modifier change in VoxelGrid

Julian Löchner
After a long time, I just wanted to finally do this. Problem is, searching PCL 1.5.1 source code for .h-files containing "using", 190 files are found. Clearly, not in all cases "using" is used in this particular way. But very often it is used in such a way that it changes the access level of the corresponding variable. This is the case for most filter classes, most surface classes and probably for others as well (e.g. see my first post on this topic). I don't have the time to check all 190 files and fire up individual issues. I actually don't understand why the "using" keyword is used at all everywhere.

Cheers, Julian
Reply | Threaded
Open this post in threaded view
|

Re: Access control modifier change in VoxelGrid

Radu B. Rusu
Administrator
Julian,

What is the question? :) Why do we use "using" for templated inheritance? Simply because you don't want to open up a
file that should contain an algorithm and see things like:

PCLBase<PointInT, PoinTNT, PointOuT>::input_->points[(*PCLBase<PointInT, PoinTNT, PointOuT>::indices_)[i]]

in the code. What you want to see is:

input_->points[(*indices_)[i]]

at best.

Cheers,
Radu.
--
http://pointclouds.org

On 03/28/2012 03:58 AM, Julian Löchner wrote:

> After a long time, I just wanted to finally do this. Problem is, searching
> PCL 1.5.1 source code for .h-files containing "using", 190 files are found.
> Clearly, not in all cases "using" is used in this particular way. But very
> often it is used in such a way that it changes the access level of the
> corresponding variable. This is the case for most filter classes, most
> surface classes and probably for others as well (e.g. see my first post on
> this topic). I don't have the time to check all 190 files and fire up
> individual issues. I actually don't understand why the "using" keyword is
> used at all everywhere.
>
> Cheers, Julian
>
>
> --
> View this message in context: http://www.pcl-developers.org/Access-control-modifier-change-in-VoxelGrid-tp4823298p5600099.html
> Sent from the Point Cloud Library (PCL) Developers mailing list archive at Nabble.com.
> _______________________________________________
> PCL-developers mailing list
> [hidden email]
> http://pointclouds.org/mailman/listinfo/pcl-developers
> http://pointclouds.org
_______________________________________________
PCL-developers mailing list
[hidden email]
http://pointclouds.org/mailman/listinfo/pcl-developers
http://pointclouds.org
Reply | Threaded
Open this post in threaded view
|

Re: Access control modifier change in VoxelGrid

Julian Löchner
true point. But the change of the access level (I mean private, public, protected) in the derived classes is confusing too. Attributes and methods that were protected or public in the base class are suddenly changed to private in a derived class.
Reply | Threaded
Open this post in threaded view
|

Re: Access control modifier change in VoxelGrid

Radu B. Rusu
Administrator
Julian,

That's what I meant earlier: we should treat them on a case by case basis and see where we failed. Some developers
probably didn't want their classes subclassed, while others simply omitted to implement the correct access modifiers.

Cheers,
Radu.
--
http://pointclouds.org

On 03/28/2012 09:39 AM, Julian Löchner wrote:

> true point. But the change of the access level (I mean private, public,
> protected) in the derived classes is confusing too. Attributes and methods
> that were protected or public in the base class are suddenly changed to
> private in a derived class.
>
> --
> View this message in context: http://www.pcl-developers.org/Access-control-modifier-change-in-VoxelGrid-tp4823298p5601112.html
> Sent from the Point Cloud Library (PCL) Developers mailing list archive at Nabble.com.
> _______________________________________________
> PCL-developers mailing list
> [hidden email]
> http://pointclouds.org/mailman/listinfo/pcl-developers
> http://pointclouds.org
_______________________________________________
PCL-developers mailing list
[hidden email]
http://pointclouds.org/mailman/listinfo/pcl-developers
http://pointclouds.org
Reply | Threaded
Open this post in threaded view
|

Re: Access control modifier change in VoxelGrid

Michael Dixon-2
In reply to this post by Julian Löchner
> true point. But the change of the access level (I mean private, public,
> protected) in the derived classes is confusing too. Attributes and methods
> that were protected or public in the base class are suddenly changed to
> private in a derived class.

I believe most (if not all) of the access level changes in the derived
classes are simply mistakes.  It looks like a lot of our developers
(myself included!) were careless about where we put the "using"
statements, but it should be easy to fix.  Do you have a list of which
classes are the biggest offenders?

-Michael
_______________________________________________
PCL-developers mailing list
[hidden email]
http://pointclouds.org/mailman/listinfo/pcl-developers
http://pointclouds.org
Reply | Threaded
Open this post in threaded view
|

Re: Access control modifier change in VoxelGrid

Julian Löchner
besides the ones I already mentioned, no. Best idea I have is to search the library for "using" and check all 190 resulting files...