Richard Heck wrote:
Richard Heck wrote:
Abdelrazak Younes wrote:
Abdelrazak Younes wrote:
Richard Heck wrote:
This got started because there was a crash that involved passing a
temporary to Paragraph::setLayout(), and I added a comment to the
effect that one ought not to do that. Then Andre suggested there
must be a better way, in particular, that maybe we could make
Layout's constructor private, etc. Unfortunately, you can't do
that, because we have a std::vector of Layout's.
What's wrong with a vector<Layout *> and a private Layout ctor? You
just need to declare TextClass as friend in the layout class, that's
all. I probably missed something...
I had a look at TextClass, I think that the public typedef
(LayoutList and const_iterator) are not really needed. They are only
used in GuiToolbars.cpp and GuiDocument.cpp; we can write TextClass
helper methods for those cases.
Then, if you don't need the public iterator interface, there's
nothing stopping us from using vector<Layout *>, AFAICS from q quick
glance.
That's pretty much how the code used to be. Maybe I'm just discovering
why it was like that. But what we DON'T need here is the shared
pointer that used to be used: It was
std::vector<boost_shared_ptr<Layout> >.
That said, the iterator is really natural, and it was when I wanted to
use one rather than to use some sort of helper than things started
getting messy.
See http://www.lyx.org/trac/changeset/23505#file2.
Maybe there's a better way to do this than by indexing into the
LayoutList. I didn't like that, and I didn't like the way these indices
were being used in GuiDocument. In effect, it means that GuiDocument has
to know that we're implementing the LayoutList as a vector. But maybe we
don't want to do that at all. Maybe LayoutList should really be a map
from names to Layouts. After all, we generally access these things by
their names.
Right, if Layout access is not performance critical, we should switch to
a map. GuiDocument is on the dialog side and by definition, dialogs are
not performance critical.
And now that we're not just listing the layouts in the
order they were created---they're sorted into categories, or sorted by
their title, or whatever. So, anyway, that's what I was trying to
change, by using an iterator. But maybe, as you said, there is some
other way to do it.
map<name, Layout *> seems like a good solution to me.
Abdel.
