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

[Julien Nabet]

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