Question about sc/source/core/data/colorscale.cxx

Markus Mohrhard markus.mohrhard at googlemail.com
Sat Dec 15 06:39:26 PST 2012


Hey,

> I noticed these lines in file sc/source/core/data/colorscale.cxx
> (because cppcheck report indicates that bNeedUpdate is reassigned before to
> be used line 641)
>
>     637 void ScDataBarFormat::DataChanged(const ScRange& rRange)
>     638 {
>     639     bool bNeedUpdate = false;
>     640
>     641     bNeedUpdate = NeedUpdate(mpFormatData->mpUpperLimit.get());
>     642     bNeedUpdate &= NeedUpdate(mpFormatData->mpLowerLimit.get());
>     643
>     644     bNeedUpdate &= GetRange().Intersects(rRange);
>     645
>     646     if(bNeedUpdate)
>     647     {
>     648         mpDoc->RepaintRange(GetRange());
>     649     }
>     650 }
>
> First I wonder if we shouldn't avoid to bitwise and and logical and. But
> above all, shouldn't it be logical OR?

Only for the first one. For the second one the OR is totally wrong and
will force always a repainting which is a preformance nightmare.

>
> So I would rather put:
>  if( (NeedUpdate(mpFormatData->mpUpperLimit.get()))
> || (NeedUpdate(mpFormatData->mpLowerLimit.get())) ||
> (GetRange().Intersects(rRange)) )
> {
>     mpDoc->RepaintRange(GetRange());
> }
>
> What do you think?

Implemented a correct version with 4f901a2f451a552853c1dd38309dd55b22616fdd.


More information about the LibreOffice mailing list