Re: [PATCH] blame.c: prepare_lines should not call xrealloc for every line

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> David Kastrup <dak@xxxxxxx> writes:
>
>> It's snake oil making debugging harder.
>
> OK, that is a sensible argument.
>
>>> This was fun ;-)
>>
>> At the expense of seriously impacting my motivation to do any further
>> code cleanup on Git.
>
> Well, I said it was "fun" because I was learning from a new person
> who haven't made much technical or code aethesitics discussion here
> so far.  If teaching others frustrates you and stops contributing to
> the project, that is a loss.

The implication of "Thanks for catching them, but this patch needs heavy
style fixes." is not one of learning.

> But the styles matter, as the known pattern in the existing code is
> what lets our eyes coast over unimportant details of the code while
> reviewing and understanding.  I tend to be pickier when reviewing code
> from new people who are going to make large contributions for exactly
> that reason---new blood bringing in new ideas is fine, but I'd want to
> see those new ideas backed by solid thiniking, and that means they may
> have to explain themselves to those who are new to their ideas.

Well, the point of stylistic decisions is exactly that they are not
individually "backed by solid thinking": if they were, one would not
require a style.

A pattern of
some loop {
  ...
  if (condition) {
    code intimately related to condition;
    continue;
  }
  break;
}

is somewhat awkward.  The superficially simpler

some loop {
  ...
  if (!condition)
    break;
  code intimately related to condition;
}

separates related parts with a central loop exit.  Which maybe a tossup.
The former pattern of break; at the end of a loop, however, becomes
indispensible in the form

some loop {
  ...
  switch (...) {
    various cases ending in either break; or continue;
  }
  break;
}

In this case, the break; before the end of the loop establishes the
statement "any commencement of the loop will be explicitly done using
continue;", particularly important since a break; inside of the switch
statement does not, without this help, break out of the loop.

It's a pattern that is transparent enough to be still preferable over
"crank out the goto already, chicken".

Is "if I have to use x in some situations anyway I may as well pick it
when there would be equivalent solutions" solid thinking?  Not really.
It's about choosing one's familiars.  Which ultimately boils down to
personal style.  And where the differences are not really significant
for understanding and _are_ a conscious expression rather than just an
accident, the thin line between "write in a way that does not go against
our style and/or good sense" and "write in the way I would have written
it" may be the line between fun and work.

-- 
David Kastrup

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]