re: #28960 (review)
Pointer-to-implementation is a c++ specific pattern. It’s a simple trick to divide a single class into two halves so that the private parts of the class are in the .cpp file rather than the .h file. It doesn’t have to do with SOLID principles or object oriented design.
The typical way to define a class in c++ is
0class MyClass
1{
2public:
3 // MyClass public methods and members
4private:
5 // MyClass private methods and members
6};
This works well in most cases, but in some cases you don’t want to expose private parts of the class in the header. For example if you are trying to avoid adding private dependencies to the header, or trying to define a stable ABI. In these cases, you can use the pointer to implementation trick to divide MyClass into two parts, with the public part in the header file:
0class MyClass
1{
2public:
3 // MyClass public methods only
4private:
5 std::unique_ptr<MyClassImpl> m_impl;
6};
And the private part in the cpp file:
0class MyClassImpl
1{
2 // MyClass private members and methods.
3};
The problem with f442a3a5b2a0a58bc263fbb9c87e8e4715de103a is that it is not following either of these two approaches, but instead taking a hybrid approach when private state is split up across both the public and private classes. This is bad, because if it means that the private methods only have access to part of the private state, not all of it.
In OO terms, ValidationSignals and ValidationSignalsImpl do not represent separate classes. They just represent the public and private parts of a single class, that are declared in separate halves because of C++ headers weirdness that doesn’t exist in other languages.
If you actually think that ValidationSignals and ValidationSignalsImpl should be separate OO classes, I think you will want to rename the ValidationSignalsImpl to reflect the actual role of this class rather than taking the name of the class which contains it.
