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

Hello Markus

Le Sat, 15 Dec 2012 15:39:26 +0100, Markus Mohrhard  
<markus.mohrhard@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