On Tuesday 2007 February 20 10:30, Junio C Hamano wrote:
> If we _were_ doing the inline function, I would actually prefer:
>
> static inline ref_is_head(const char *ref)
> {
> return !prefixcmp(ref, PATH_REFS_HEADS);
> }
As you brought it up...
I've never really liked "!" on strcmp() lines (but I accept that that is the
tradition in git) because it implies the the output of prefixcmp is boolean,
but it's actually ternary. strcmp() (I think), should be thought of as
outputting
enum {
STRING1_LESS_THAN_STRING2,
STRINGS_EQUAL,
STRING1_GREATER_THAN_STRING2
}
Given that, it makes me uncomfortable to use !strcmp(). Of course in the case
of strcmp(), that form is so well known that it makes very little difference
to the reader.
I have similar feelings about
if( !something )
being incorrect when you meant
if( something == NULL )
While they are identical in what they generate, they send a different message
to someone reading the code.
Regardless, I'm not so stubborn as to refuse to go with the flow...
> But at least to me,
>
> if (!prefixcmp(head, PATH_REFS_HEADS))
> head += strlen(PATH_REFS_HEADS);
>
> is easier to follow than:
>
> if (ref_is_head(head))
> head += STRLEN_PATH_REFS_HEADS;
Fine. I don't really mind - and it's less work on my patch :-)
My argument in favour of the ref_is_head() method is that the prefixcmp()
method requires knowledge from the caller about how you tell whether a given
ref is a head - the second pushes that information further down the call
tree, abstracting it out just a little more.
As I say though - it's not a problem for me.
Andy
--
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@xxxxxxxxx
-
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