[PATCH] [core/vcl/source/window/splitwin.cxx:2047] ->[core/vcl/source/window/splitwin.cxx:2045]:(style) Found duplicate branches for if and else.

Matteo Casalin matteo.casalin at gmx.com
Thu Feb 16 03:18:48 PST 2012


Hi all,

--------------------------------------------------
Mariusz Dykierek <mariuszdykierek at gmail.com> wrote:
(16/02/2012 11:46)

> On 2012-02-16 10:02, Stephan Bergmann wrote:
> > On 02/16/2012 09:35 AM, Riccardo Magliocchetti wrote:
> >> Otherwise you can simplify it even more:
> >>
> >> sal_Bool bLeft = (meAlign == WINDOWALIGN_TOP || meAlign ==
> >> WINDOWALIGN_LEFT) ? sal_False : sal_True;
> >
> > ... which of course reduces to
> >
> >> bool bLeft = !(meAlign == WINDOWALIGN_TOP || meAlign == WINDOWALIGN_LEFT);
> >
> > or
> >
> >> bool bLeft = meAlign == WINDOWALIGN_RIGHT || meAlign == WINDOWALIGN_BOTTOM
> >
> > given that WindowAlign has exactly those four members (and it makes the name "bLeft" look
> > odd...).
> >
> I personally find 'if' more legible than ?: and definitely expressions like b = x==y || x==z;
> I am not sure if WindowAlign will always have only these 4 members and possibly the author of
> the original version wasn't either (thus final else).
> 
> Since I am a newbie here, I would vote for a simple 'if' or a 'switch'.
> Let me know what is the decision and I will change the code accordingly or... feel free to
> change the code and provide an alternative patch.

+1 for switch/break approach!
More legible and open to possible later modifications without 
requiring "code decodification" efforts. Hopefully the compiler is 
smart enough to generate efficient code for such simple actions.

Best regards
Matteo

> _______________________________________________
> LibreOffice mailing list
> LibreOffice at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice



More information about the LibreOffice mailing list