Re: Bugfix for issue 1630 (issue4490045)

[Benkő Pál]

2011/5/28  <address@hidden>:
> On 2011/05/28 16:13:43, benko.pal wrote:
>>
>> aargh, that's not too readable.
>> what I actually suggest is replacing lines 204-207 of
>
>> >
>
> http://codereview.appspot.com/4490045/diff/12001/lily/completion-note-heads-engraver.cc
>>
>> > File lily/completion-note-heads-engraver.cc (right):
>
>> 204       if ((left_to_do_ - note_dur.get_length ()) > Rational (0))
>> 205         event->set_property("autosplit-end", ly_bool2scm (true));
>> 206       else
>> 207         event->set_property("autosplit-end", ly_bool2scm (false));
>
>> by
>
>>  &nbsp; event->set_property ("autosplit-end",
>>  &nbsp; &nbsp; ly_bool2scm (left_to_do_ - note_dur.get_length () >
>
> 0));
>
>> Pal
>
> That was the original code.  It was pointed out (see Neil's comment
> above) that the only check on this is whether or not it is greater than
> zero, so a boolean works.  Hence, the code was changed to use a boolean.

I must miss something, to me it's still a boolean.
and (still to me) it's not an inline conditional, but
an assignment of a boolean expression to a boolean
variable.

to me the idiom

if (complex_boolean_expression)
  foo(true);
else
  foo(false);

is more cluttersome than

foo(complex_boolean_expression);

but if that's a minority opinion, then leave Karin's
code as is, of course.

> http://codereview.appspot.com/4490045/