Abdelrazak Younes wrote:
rgheck wrote:
Andre Poenitz wrote:
On Mon, Feb 25, 2008 at 04:50:42PM -0500, rgheck wrote:
I personally don't like this TextClassPtr class. 'TextClass *' is
meaningful enough. I would suggest to get rid of the TextClassPtr
header altogether now.
That may be the thing to do. But let me explain my thinking further.
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.
But I understand now that a TextClass is mandatory now, isn't it? So
using a reference instead of a pointer is better. Pointers are good if
the referenced instance is not mandatory. For example
GuiView::current_work_area_ is a pointer but BufferView::buffer_ is a
reference.
This is actually independent of the question whether to have a
TextClassPtr thingy. If we have it, maybe it should wrap
std::list<TextClass> instead of std::list<TextClass *>.
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.
Why? If a TextClass is mandatory, give it the default one at
construction IMHO. So a reference would work as well.
This could be done, but it's not so much a TextClass that is mandatory
but a particular kind of TextClass, one that (a) can be guaranteed never
to be destroyed (or, at least, never to be destroyed while anything
still has a reference to it) and (b) represents the layout for a buffer,
as opposed (say) to representing the information from a layout file. The
first is why I was previously using boost:shared_ptr and the latter is
why InsetCollapsable's constructur looks like this:
InsetCollapsable(
BufferParams const &,
CollapseStatus status = Inset::Open,
TextClassPtr tc = TextClassPtr()
);
If we just had TextClass & there, say, then it would be possible to do
this, too:
InsetCollapsable(buf, Inset::Open, baseclasslist[baseClass_]);
which we do NOT want to do. We don't want TextClasses from that list
doing that work.
Maybe a better solution then would be a fairly simple subclass of
TextClass that could appear in this sort of place. The trick of making
the constructor private so that only TextClassBundle can make these
things could still be used then. Does that seem more straightforward?
rh
