Re: [PATCH] diff: round down similarity index

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

David Kastrup schrieb:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>>
>>> +static int similarity_index(struct diff_filepair *p)
>>> +{
>>> +	int result = p->score * 100.0 / MAX_SCORE;
>>> +
>>> +	/* Paranoia: guard against floating point rounding errors. */
>>> +	if (p->score == MAX_SCORE)
>>> +		result = 100;
>>> +	else if (result == 100)
>>> +		result = 99;
>>> +
>>> +	return result;
>>> +}
>> Why not simply do this?
>>
>>         static int similarity_index(struct diff_filepair *p)
>>         {
>>                 if (p->score == MAX_SCORE)
>>                         return 100;
>>                 return p->score * 100 / MAX_SCORE;
>>         }
>>
>> MAX_SCORE and p->score run up to 60000 and we assume int is at
>> least 32-bit, so I do not think there is no risk of overflowing.

Good idea!  *smacks himself against the forehead*

I just copied the old code (sans the addition of 0.5) and then tried
to work around it's rounding problem (which I'm not even certain
exists and matters).  Look out for an updated patch..

> You bet me to it on the way to work.  Anyway, when working with
> integers, the first two lines are definitely not required (I am
> assuming that p->score is integer as well but have not checked).

Right.

> What probably requires more thought is the calculation of p->score
> itself: if one has large numbers as a ratio (original vs changes
> lines), multiplying by 60000 before dividing by the second large
> numbers might exceed the size of the integers.

MAX_SCORE is defined as 60000.0, which makes it a double.  AFAICS all
the multiplications with MAX_SCORE are done using floating point
arithmetic for that reason.

René