Andre Poenitz wrote:
On Mon, Feb 25, 2008 at 04:50:42PM -0500, rgheck wrote:
In my last commit regarding boost::shared_ptr, I left TextClassPtr as a
silly typedef:
typedef TextClass * TextClassPtr;
The attached would turn it into something akin to a "strong typedef", where
TextClassPtr really just wraps a TextClass*. The point of this is to
enforce the distinction between TextClasses generally, which can represent
various things (see the comment in TextClass.h), and TextClasses that go
with Buffers. A TextClassPtr is thus what is held in the TextClassBundle,
which just holds pointers to the TextClasses that are associated with
Buffers. This (a) keeps them alive even when the Buffer itself is deleted
but (b) keeps them from being leaked (though we're never, at the moment,
deleting them---that's the cost of abandoning boost::shared_ptr---though
maybe we could).
If this seems like over-engineering, well, I'm sure it is, a bit. On the
other hand, looking back through this code again has made me think the
differences between the various things TextClasses represent need to be
made more apparent, and this seems like a good way to do it. In the code as
it now stands, all access to the TextClasses that go with Buffers is via
TextClassPtr, but this doesn't enforce anything. With this new class, we do
require that all such TextClasses be represented by TextClassPtrs, and we
restrict creation of such TextClasses to TextClassBundle itself. So the
compiler will help us keep from confusing things.
Comments welcome.
How many of those TextClass beasts appear in a typical LyX session?
Two? A dozen? Hundreds?
Maybe half a dozen. So it's not a lot.
Can't TextClassBundle not just be a std::list<TextClass>
This might be possible, but there are cases (such as in
InsetCollapsable) where we have (and did have, before my stuff) default
arguments like: TextClassPtr tc = 0. And there are classes, like
BufferParam, but others too, that have TextClassPtr's as members, where
it's more sensible to have a null pointer as initial object.
And we don't want TextClassBundle just to BE a std::list<TextClass>, as
a typedef. (Maybe that's not what you were suggesting.) The idea here
was that we (programmers) would then have to remember always to add the
TextClass to the list if we constructed it, which of course we'll forget
to do.
and instead of TextClassPtr we use real references to items in that list?
You'll have to help me out here. BufferParams holds a something to its
TextClass---presently a pointer---and that is the main use of this
structure. Can it hold a reference: viz, private: TextClass &
text_class_? If not---and I don't remember ever seeing this---then it
has to be a pointer, or else we have to have some other sort of index
into this list, like BaseClassIndex. (Prior to my work, that's exactly
what this was: text_class_ was just textclass_type = int.) But then
TextClassPtr is just an alternative sort of implementation, but one that
makes usage more natural, to my mind. BaseClassIndex always has to be
dereferenced via something like:
baseclasslist[BaseClassIndex]
whereas a TextClassPtr acts like a pointer, because that's what it is.
Richard
