Re: Redefine semantics of find_unique_abbrev()

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 2, 2008, 08:11 UTC
Message-ID: <7vtzjpsfz5.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080302074230.GD3935@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 27 quoted lines

> On Sat, Mar 01, 2008 at 11:35:11PM -0800, Junio C Hamano wrote:
>
>> That is, "for objects we have, make sure it uniquely identifies,
>> otherwise, make sure the phoney name is long enough such that it would not
>> name any existing object".
>
> I think your logic is correct, and I think naming it 'exists' is more
> readable (I don't have a tendency not to double-negate).
>
> But...
>
>> -		if (!status ||
>> -		    (is_null && status != SHORT_NAME_AMBIGUOUS)) {
>> +		if (exists
>> +		    ? !status
>> +		    : status == SHORT_NAME_NOT_FOUND) {
>>  			hex[len] = 0;
>>  			return hex;
>>  		}
>
> Maybe it is just me, but I find the ternary operator here reduces
> readability. I would have liked the more verbose:
>
>   if ((exists && !status) ||
>       (!exists && status == SHORT_NAME_NOT_FOUND)) {
>
> But now I am just painting your bikeshed.

Heh, the ternary is a mini "if-then-else" by itself. Turn your head sideways (just like you do when you meet a smiley) and the parse tree will jump at you ;-).