Re: [Libreoffice] [PATCH] Remove unnecessary parameter to IDocumentStatistics::UpdateDocStat

hello,

I will see this week end. UpdateDocState already check if the stats are
updated, so we can run UpdateDocStat all times I think.

Thanks for the review

2011/8/19 Caolán McNamara <caolanm@redhat.com>

> On Wed, 2011-08-17 at 21:04 +0200, Arnaud Versini wrote:
> > Hi
> >
> > This patch remove unnecessary parameter SwDocStat& rStat to the method
> > IDocumentStatistics::UpdateDocStat and also limit copy of SwDocStat
> > during file saving and statistic dialog.
>
> There's a bug in the last hunk anyway,
>
>         {
>             SwDocStat aStat(pDocShell->GetDoc()->GetDocStat());
>             if(aStat.bModified)
> -                pDocShell->GetDoc()->UpdateDocStat( aStat );
> +                pDocShell->GetDoc()->UpdateDocStat();
>
> i.e. the "aStat" is a copy so after the UpdateDocStat is called, it's an
> unchanged copy, not a reference to an updated SwDocStat like it used to
> be, so a line or two later nValue = aStat.nChar will be wrong.
>
> Is there a specific problem you're trying to fix here ?
>
> Looking through the DocStat stuff it looks a bit messy. Seems of it
> appears to want to give access to the document statistics for a fast
> query by some code that the stats need to be updated, and by other code
> just to get the stats. And some bits fail to check for bModified and
> force an update and some do. And the awesome rtf filter const_casts the
> document doc stat and updates it directly.
>
> If we want to refactor this then I suggest...
>
> a) move "bModified" out of the SwDocStat and add something like bool
> areStatsStale/statsNeedUpdating to IDocumentStatistics instead to query
> this fact
> b) have IDocumentStatistics::GetDocStat always return updated document
> stats instead of having to get them, find out if they are stale and
> update them if they are.
>
> That would probably avoid the problems of needing to directly call
> UpdateDocStat for the general case where someone just wants to get
> up-to-date stats, but allow any special cases of calling UpdateDocStat
> when areStatsStale is true in order to force update of fields that
> display the doc stats.
>
> C.


-- 
Arnaud Versini