[Libreoffice-commits] .:

[Lubos Lunak]

On Tuesday 21 of February 2012, Stephan Bergmann wrote:
> On 02/21/2012 08:57 PM, Thomas Arnhold wrote:
> > commit 45ad6029e1422c659e627bb30ecda50ccb61060c
...
> > @@ -174,12 +174,7 @@ static sal_Bool lcl_IsLess( const ScDPDataMember*
> > pDataMember1, const ScDPDataMe sal_Bool bError1 = pAgg1&& 
> > pAgg1->HasError();
> >       sal_Bool bError2 = pAgg2&&  pAgg2->HasError();
> >       if ( bError1 )
> > -    {
> > -        if ( bError2 )
> > -            return false;       // equal
> > -        else
> > -            return false;       // errors are always sorted at the end
> > -    }
> > +        return sal_False;       // equal
>
> Just FYI, there should ~never be a need to go back from "false" to
> "sal_False" (we generally want to move any uses of the latter to become
> uses of the former; so if you seek consistency across a function, it
> might be better to replace other occurrences of sal_True/False than to
> do the opposite).

 Moreover, although the change technically keeps the code the same, it 
slightly breaks the semantics - 'if( bError1 )' alone does not mean the two 
are equal, so now it is misleading. I'll put the proper comment back.

-- 
 Lubos Lunak
 l.lunak at suse.cz