lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

flower: Add boolean return value to 'Rational' class. (issue 581400043 b


From: dak
Subject: flower: Add boolean return value to 'Rational' class. (issue 581400043 by address@hidden)
Date: Sun, 29 Dec 2019 03:23:56 -0800

Frankly, I am disconcerted that the original code runs through a float
conversion in the first place.  This is not as much about removing a
warning rather than fixing bad code since the warning had a point.


https://codereview.appspot.com/581400043/diff/577250043/flower/rational.cc
File flower/rational.cc (right):

https://codereview.appspot.com/581400043/diff/577250043/flower/rational.cc#newcode50
flower/rational.cc:50: else if (sign_ == -1 || sign_ == 1
That's unreadable and "assert (false)" is not a useful assertion to
print out.  Instead this should not even contain an else if branch
(which is entirely pointless if the if branch ends in return).

The whole function body could just be
   assert (sign_ >= -2 && sign <= 2);
   return sign_ != 0;

but I don't think the assertion is even warranted.  Why check for stuff
that the function doesn't need to work?

https://codereview.appspot.com/581400043/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]