> Great work! Is there something I should specifically look out when
> testing or would it be fione to know if things run at all already?
>
Mostly just looking for anything obvious that I may have missed, or
concerns with the implementation.
> Get a lot of warnings like this one currently:
>
> > Schweregrad Code Beschreibung Projekt Datei Zeile
> > Unterdrückungszustand
> > Warnung C4251 "log4cxx::helpers::FileInputStream::m_priv": class
> > "std::unique_ptr<log4cxx::helpers::FileInputStream::FileInputStreamPrivate,std::default_delete<log4cxx::helpers::FileInputStream::FileInputStreamPrivate>>"
> > erfordert eine DLL-Schnittstelle, die von Clients von class
> > "log4cxx::helpers::FileInputStream" verwendet wird
> > C:\Users\tschoening\Documents\Svn\Src\Libs\trunk\C++\Logging\log4cxx\master\src\out\build\x64-Debug\src
> >
> > C:\Users\tschoening\Documents\Svn\Src\Libs\trunk\C++\Logging\log4cxx\master\src\src\main\include\log4cxx\helpers\fileinputstream.h
> > 40
>
I'm going to guess that is something about not being able to use the
class across a DLL boundary. That's /probably/ not an issue for
reasons that I'll get into in a moment.
> Should this be the following instead? Looks like a copy&paste error.
>
> > std::unique_ptr<ParentPrivate> m_priv;
>
Yeah that's probably just a copy/paste error.
> Some naming discussion: How about using "ParentPriv" and
> "ParentPriv.h" instead of heaving "ParentPrivate", "Parent-private.h"
> and "m_priv". It's shorter, "-private" doesn't follow QT conventions
> anyway and the abbreviation "priv" is used already as well.
>
That sounds reasonable to me.
> When looking at your code, you are using some "XyPriv" and "xy_priv.h"
> and some "XyPrivate" and "xy_priv.h" already. For existing classes, I
> suggest keeping "xy_priv.h", but for newer classes "XyPriv" and
> "xyPriv.h" read better in my opinion. So I would suggest that in your
> new docs.
>
> About inheritance: Wouldn't it be better in most cases to provide
> protected or public getters in base classes anyway instead of
> accessing private data structs?
>
> I understand your example to show how it could be done in case no such
> getters are available or should be used for some reason. Or is that
> example showing how it needs to be done always regardless how the
> private data is accessed by subclasses?
>
The example shows how it needs to be done anyway. The reason for this
is explained a bit more in the Qt documentation, but here's a simple
explanation:
If we have a class with private data:
class A {
unique_ptr<APrivate> a_priv;
}
And we then need to subclass it:
class B : public A{
unique_ptr<BPrivate> b_priv;
}
Whenever we create class B, we now do two memory allocations(APrivate
and BPrivate). In the code for B, you then also need to keep track of
which superclass actually holds the data. If we instead subclass
'APrivate' in 'BPrivate', we can then store the pointer to 'BPrivate'
in the 'A' class.
struct APrivate{
int32_t a;
};
struct BPrivate : public APrivate{
int32_t b;
};
BPrivate is now 8 bytes in size. Because it is a subclass of
APrivate, we can store the pointer to BPrivate inside of A, thus
saving us a memory allocation and letting us pretend that we still
have a "normal" inheritance:
class A{
unique_ptr<APrivate> priv;
};
class B: public A{
B(){
priv = make_unique<BPriv>(); // 'priv' is of type APrivate, so
must downclass in B whenever we want to use it
}
};
> > #define priv static_cast<AndFilterPrivate*>(m_priv.get())
>
> Does it make sense to mention something like that in your new docs?
> Is it possible to make it a (generic?) getter or alike instead of a
> macro?
Unfortunately not, for the reasons above. Since the parent class only
holds a single pointer to the private data, whatever data is stored
must be a subclass of the parent's private data. So we must cast it
to whatever class's private data that we are currently using.
Because of this as well, the 'private' classes don't actually need to
be exported out of the library: you only need them if you are
subclassing an object and you are interested in doing only one memory
allocation for your private instance variables.
-Robert Middleton
