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

[Mat M]

Hello Markus

Le Sat, 15 Dec 2012 15:39:26 +0100, Markus Mohrhard  
<markus.mohrhard at googlemail.com> a écrit:

> 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.

Sorry but what I see in 4f901a2f45 is the same thing as Julien pasted in  
his mail.
It rollback the good version:

$ git diff  
4f901a2f451a552853c1dd38309dd55b22616fdd..4f901a2f451a552853c1dd38309dd55b22616fdd^
diff --git a/sc/source/core/data/colorscale.cxx  
b/sc/source/core/data/colorscale.cxx
index f5d23c1..2adf507 100644
--- a/sc/source/core/data/colorscale.cxx
+++ b/sc/source/core/data/colorscale.cxx
@@ -636,8 +636,10 @@ bool NeedUpdate(ScColorScaleEntry* pEntry)

  void ScDataBarFormat::DataChanged(const ScRange& rRange)
  {
-    bool bNeedUpdate = NeedUpdate(mpFormatData->mpUpperLimit.get());
-    bNeedUpdate |= NeedUpdate(mpFormatData->mpLowerLimit.get());
+    bool bNeedUpdate = false;
+
+    bNeedUpdate = NeedUpdate(mpFormatData->mpUpperLimit.get());
+    bNeedUpdate &= NeedUpdate(mpFormatData->mpLowerLimit.get());

      bNeedUpdate &= GetRange().Intersects(rRange);

Could you check if I missed something ?

Regards
-- 
Mat M