Re: [Libreoffice] Possible little code simplification

On 25/11/11 19:05, Ivan Timofeev wrote:
> Hello Michael,
>
> 2011/11/25 Michael Stahl <mstahl@redhat.com>:
> > On 24/11/11 19:07, Ivan Timofeev wrote:
> > > [...]
> > > What do you think about changing the code so that pSortedObjs will be
> > > alive every time? We will be able to return a reference to pSortedObjs
> > > in GetSortedObjs() instead of a pointer and remove the mentioned
> > > checks (there are a lot of such checks). I could do this. :-)
> >
> > the writer core and layout code is highly optimized, especially for
> > 16-bit windows where memory is scarce :)
>
> 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 :)

> > i guess for SwPageFrm it doesn't matter at all if the SwSortedObj always
> > exists.
> >
> > > Same problem with the field 'SwSortedObjs* pDrawObjs' in the class SwFrm.
> >
> > since this is the base class of all frames there will be a couple more
> > instances of it, and most of these probably won't use pDrawObjs; would
> > be interesting to know how much memory this would uses for a large
> > document...
>
> 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 :)

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.

> Kind Regards,
>     Ivan

regards,
 michael