Re: Shortcut Rational addition if either operand is zero (issue 551690046 by address@hidden)

[nine . fierce . ballads]

LGTM


https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc
File flower/rational.cc (right):

https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc#newcode257
flower/rational.cc:257: if (is_infinity ())
The hazard of reviewing this class is that there's always something to
wonder about.  This time, I wonder about adding +infinity and -infinity
together (in either order).

https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc#newcode261
flower/rational.cc:261: else if (r.sign_ == 0)
Is this the only micro-optimization you measured?  Other ideas you might
wish to try:

* move the conditions that read this->sign_ together

* are infinities handled first because they are significantly more
common than zeros?  try moving the branches that test for zero before
the branches that test for infinities

* inline a subset of the shortcuts in rational.hh

In any case, noting your findings in a concise comment would help future
maintainers avoid erasing your gains.

https://codereview.appspot.com/551690046/