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

On 15/12/2012 15:39, Markus Mohrhard wrote:
> 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.
Ok. I thought that 1 NeedUpdate should have triggered a RepaintRange. 
The function really should be called NeedPerhapsUpdate (just kidding of 
course :-))
> 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.
Thank you Markus for the patch and for your feedback.

Julien