Thread (1 message) 1 message, 1 author, 2020-09-10

Re: [PATCH 8/8] worktree: simplify search for unique worktree

From: Junio C Hamano <hidden>
Date: 2020-09-10 21:10:02

Eric Sunshine [off-list ref] writes:
quoted
Thanks for commenting. I found the original trickier than it had to be.
It spreads out the logic in several places and is careful to short-cut
the loop. My first thought was "why doesn't this just use the standard
form?". But I'm open to the idea that it might be a fairly personal
standard form... If there's any risk that someone else comes along and
simplifies this to use a `nr_found` variable, then maybe file this under
code churning?
Maybe. Dunno. Even with the suggested function comment, I still find
that the revised code has a higher cognitive load because the reader
has to remember or figure out mentally what `found` contains by the
`return found;` at the end of the function.
Judging from the phrase "standard form", I have a feeling that
Martin thinks that the updated code is more intuitive (i.e. "we see
another one while keeping the one we already found, so we know there
is no unique answer").  I do not claim that would be the standard way
and using a counter clamped to 2 is substandard, but I find the code
more readable with the patch than the original.

Even though it helps somewhat that the counter is named "nr_found"
and not "nr_match", I found it a bit awkward that the counter in the
original pretends to count all the answers, yet does not really
count all of them and stops at 2.

I agree with Martin that this would probably be subjective.

Thanks.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help