Re: Redefine semantics of find_unique_abbrev()

From: Jeff King <peff@peff.net>
Date: Mar 2, 2008, 07:42 UTC
Message-ID: <20080302074230.GD3935@coredump.intra.peff.net>
In-Reply-To: <7vd4qdtw7k.fsf@gitster.siamese.dyndns.org>

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...

Show 8 quoted lines

> -		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.

-Peff