[Libreoffice] Possible little code simplification

[Michael Stahl]

On 25/11/11 19:05, Ivan Timofeev wrote:
> Hello Michael,
> 
> 2011/11/25 Michael Stahl <mstahl at 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