Re: [PATCH 12/19] tree-diff: remove special-case diff-emitting code for empty-tree cases

[Junio C Hamano <gitster@pobox.com>]

Kirill Smelkov <kirr@navytux.spb.ru> writes:

>> >  static int tree_entry_pathcmp(struct tree_desc *t1, struct tree_desc *t2)
>> >  {
>> >  	struct name_entry *e1, *e2;
>> >  	int cmp;
>> >  
>> > +	if (!t1->size)
>> > +		return t2->size ? +1 /* +∞ > c */  : 0 /* +∞ = +∞ */;
>> > +	else if (!t2->size)
>> > +		return -1;	/* c < +∞ */
>> 
>> Where do these "c" come from?  I somehow feel that these comments
>> are making it harder to understand what is going on.
>
> "c" means some finite "c"onstant here. When I was studying at school and
> at the university, it was common to denote constants via this letter -
> i.e. in algebra and operators they often show scalar multiplication as
>
>     c·A     (or α·A)
>
> etc. I understand it could maybe be confusing (but it came to me as
> surprise), so would the following be maybe better:
>
>         if (!t1->size)
>         	return t2->size ? +1 /* +∞ > const */  : 0 /* +∞ = +∞ */;
>         else if (!t2->size)
>         	return -1;	/* const < +∞ */
>
> ?

Not better at all, I am afraid.  A "const" in the code usually means
"something that does not change, as opposed to a variable", but what
you are saying here is "t1 does not have an element but t2 still
does. Pretend as if t1 has a virtual/fake element that is larger
than any real element t2 may happen to have at the head of its
queue", and you are labeling that "real element at the head of t2"
as "const", but as the walker advances, the head element in t1 and
t2 will change---they are not "const" in that sense, and the reader
is left scratching his head seeing "const" there, wondering what the
author of the comment meant.

"real" or "concrete" might be better a phrasing, but I do not think
having "/* +inf > concrete */" there helps the reader understand
what is going on in the first place.  Perhaps:

        /*
         * When one side is empty, pretend that it has an element
         * that sorts later than what the other non-empty side has,
         * so that the caller advances the non-empty side without
         * touching the empty side.
         */
        if (!t1->size)
                return !t2->size ? 0 : 1;
        else if (!t2->size)
                return -1;

or something?