Re: Numeric constants as strings

On Monday 2007 February 19 09:49, Junio C Hamano wrote:

It's not primarily for that purpose, but rather to improve readability.

For example, this little bit of patch:

-       if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')
+       if (get_sha1_hex(buffer+5, tree_sha1) || buffer[HASH_WIDTH_ASCII+5] != '\n')
                return objerror(&commit->object, "invalid 'tree' line format - bad sha1");
-       buffer += 46;
+       buffer += HASH_WIDTH_ASCII+6;
        while (!memcmp(buffer, "parent ", 7)) {
-               if (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\n')
+               if (get_sha1_hex(buffer+7, sha1) || buffer[HASH_WIDTH_ASCII+7] != '\n')
                        return objerror(&commit->object, "invalid 'parent' line format - bad sha1");
-               buffer += 48;
+               buffer += HASH_WIDTH_ASCII+8;

Using HASH_WIDTH_ASCII+8 is much clearer for a reader to be able to understand intent than 48. Especially when the variables in question are called "buffer" it's harder to track what is being stored in the buffer without a named constant.

There are also a few other places where 20 is used and HASH_WIDTH_ASCII is not intended.

Andy