Re: [Libreoffice] Possible little code simplification

2011/11/28 Michael Stahl <mstahl@redhat.com>:
> On 25/11/11 19:05, Ivan Timofeev wrote:
> > Well, another possible memory optimization is to merge
> > SwSortedObjsImpl into SwSortedObjs. What is the reason to use the
> > "pImpl" pattern here?
>
> usually pImpl is a good idea because it is the only way to get actual
> encapsulation in C++; but in this case it definitely looks overdone to me...
>
> it really does not make any sense to have a separate header and cxx file
> for the impl class, that should all be moved into swsortedobjs.cxx.
>
> hmmm... am not sure about the question "should it have a pImpl at all",
> currently the impl only contains a vector, but on the other hand,
> perhaps somebody will add other members later... i don't have a strong
> opinion in this case :)

impl here is just wrapper for the vector that only inserts objects in
a right place... So, merged after all :-)

> > Ok, there are many SwFrms in a big doc... We don't want to throw away
> > memory. But we would use a shared between the SwFrm instances static
> > variable for an empty list ("null object"), and return it in
> > GetDrawObjects when our SwSortedObjs is dead. What do you think about
> > it?
>
> for this case i don't like it, because this null object would be
> mutable: it would be possible that somebody mis-uses this to get the
> pDrawObjs, receiving the static instance, and then adding an element,
> which is obviously bogus.
>
> so the explicit null checks look like the lesser evil :)

Ah yes, you are right! ;) I foolishly didn't consider that case.
Leaving it as is.

> what perhaps could be investigated is whether the pDrawObjs really has
> to be a member of SwFrm, perhaps it could be moved to some sub-class
> that is less often used... but note i am not familiar with the layout
> code so that is just a guess.

Heh, I'm afraid that I haven't got the guts to do such analysis. :-)

Thank you for your attention and tips!

All the best,
    Ivan