Clean up Filter and FilterIndices, move indices_/input_ from public to protected section

[SergioRAgostinho]

Just did some class cleanup while prototyping the new SamplingFilter class.

[taketwo]

LGTM, but where is API break?

[kunaltyagi]

filters/include/pcl/filters/filter.h doesn't export private variables anymore. It's also a fix so maybe we can add that too.

[taketwo]

Filter is an abstract class, so nobody used it directly. Doesn't this mean that nobody can be affected by this move?

[kunaltyagi]

All classes inheriting from Filter now don't have those variables available freely. Anyone deriving from Filter directly or indirectly has to modify their code IFF they use these variables.

[taketwo]

Please elaborate more, I still don't get. The following code is similar to what we have, but is not affected by access modifier change (C++ shell link):

#include <iostream>

class A {
protected:
  int foo = 1;
};

class B : public A {
public:  // changing this to protected does not break code
  using A::foo;
};

class C : public B {
public:
  using B::foo;
};

int main()
{
  C c;
  std::cout << c.foo << std::endl;
}

[kunaltyagi]

Remove using B::foo; and then change the using A::foo; to protected

[taketwo]

OK, but this only affects the "external" usage of the fields then.

Anyone deriving from Filter directly or indirectly has to modify their code IFF they use these variables.

If I understand right, the fields are still accessible in the deriving classes, it's just that they are not public.

[kunaltyagi]

the fields are still accessible in the deriving classes, it's just that they are not public

Yes, you'd be right. This does mean an API break right? Code stops compiling = API break. Code stops linking = ABI break.

[SergioRAgostinho]

Strictly speaking, this is an API breakage situation. Even if it was a mistake in the first place to expose these fields.

[taketwo]

OK, agree, this is an API break. And we skip the deprecation ritual because this was a mistake.