Re: [PATCH v4] git: replace greater-than and less-than checks with one not equal check

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 18, 2024, 15:39 UTC
Message-ID: <xmqqjzbxt2yi.fsf@gitster.g>
In-Reply-To: <pull.1432.v4.git.git.1734489859673.gitgitgadget@gmail.com>

"AreaZR via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 5 quoted lines

>  	if (top_index[0] == htonl(PACK_IDX_SIGNATURE)) {
>  		version = ntohl(top_index[1]);
> -		if (version < 2 || version > 2)
> +		if (version != 2)
>  			die("unknown index version");

I am of two minds. If the code never evolves and we will never support anything other than version #2, your rewrite certainly makes it easier to read. On the other hand, if we plan to ever learn to grok versions #3 and later, the original would be easier to se what is going on, i.e.

		if (version < VERSION_LB || VERSION_UB < version)
			die("version out of bounds");

and the code as written happens to have "2" as both lower- and upper-bound.

Of course when we do introduce version #3, this line must be updated anyway, but the final form would be as we have it with the second "2" replaced with "3", so leaving it in the current shape may be easier for the developer doing that work.

So I do not know if the proposed change is an improvement for the longer term.