Re: [PATCH 4/5] rebase -i: learn to abbreviate command names

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 3, 2017, 01:18 UTC
Message-ID: <xmqqk1y4libp.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <alpine.DEB.2.21.1.1711292236010.6482@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 8 quoted lines

> I am a fan of not relying too heavily on compiler optimization and e.g.
> extract code from loops when it does not need to be evaluated every single
> iteration. In this case:
>
> 	const char *pick = abbreviate_commands ? "p" : "pick";
> 	...
> 		strbuf_addf(&buf, "%s %s ", pick,
> 			    oid_to_hex(&commit->object.oid));

I would have called that variable "pick_cmd", not just "pick"; this preference is minor enough that I would probably reject a patch to rename from one to the other if the above were already part of the existing codebase.

I find that the code suggested above easier to follow, simply because it expresses clearly the flow of thought and that flow of thought matches how I personally think: we decide how this command is spelled in the output upfront, and then use that same spelling consistently throughout the loop.

I do not think it matters performance-wise either way, but I value how easy it is to follow the code for humans, and it matters much more in the longer run. If a compiler does a poor job, we can eventually notice and help it to produce better code that still does what we wanted it to do (or it may not be performance critical and we may not even notice). If a code is hard to follow, on the other hand, what we wanted it to do in the first place becomes harder to figure out.