On Tue, Oct 02, 2007 at 12:47:59PM -0700, Junio C Hamano wrote:
> - it makes typo harder to make and easier to spot
> (e.g. "refs/head/");
>
> - it makes miscount harder to make and easier to spot (e.g.
> what is this magic constant 11? Is it strlen("refs/heads/")?);
>
> - it makes reviewing the resulting code, and more importantly,
> future patches on the resulting code, easier.
> [...]
> It however actively hurts on the third count. These long
Yes, I find some of the substitutions more readable, but some are a bit
less readable. The parts of the patch I found the _most_ improved are
the ones that get rid of a memcmp in favor of a prefixcmp (i.e.,
removing the count entirely).
Perhaps a better quest would be to eliminate all of those counts
entirely with code that is obviously correct. I think it is much more
readable to replace:
url = xmalloc(strlen(repo->base) + 64);
sprintf(url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
with something like:
strbuf_init(&url);
strbuf_addf(&url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
which has the same number of lines, but no magic numbers at all. Or
since most of the uses of things like PATH_OBJECTS are more or less the
same, maybe something like:
mkpath_object(&url, "pack/pack-%s.idx", hex);
i.e., rather than fiddling with string constants, wrap them
functionally.
> constants in CAPITAL_LETTERS_WITH_UNDERSCORE shout too loudly to
Part of the problem is also that they're long. Perhaps REFS_HEADS, while
being less unique in the C namespace, would look better?
-Peff
-
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
