Hi, On Tue, 28 Aug 2018, Alexander Monakov wrote:
> > I think your proposed one > > warrants some comments. Maybe trade speed for some clearer code? > > I think it's not too complicated, but how about adding this comment: > > profile_count m = c1.max (c2); > /* Return 0 if counts are equal, -1 if E1 has the larger count. */ > return (m == c2) - (m == c1); > > Or, alternatively, employ more branchy checks: > > if (c1 == c2) > return 0; > if (c1 == c1.max (c2)) > return -1; > return 1; You want to at least comment on why a tradition </>/== check isn't working, namely because of the uninitialized counts. (Though I also think that comparing uninitialized counts is just non-sense and hence should actually be ruled out from the whole comparison business, at which point a normal <= check can be done again). > > The comments on the operator<> implementations are odd as well: > > (an aside, but: it also has strange redundant code like > > if (*this == profile_count::zero ()) > return false; > if (other == profile_count::zero ()) > return !(*this == profile_count::zero ()); > > where the second return could have been 'return true;') Oh my. Ciao, Michael.
