Re: Numeric constants as strings

[Andy Parkins <andyparkins@gmail.com>]

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

> I do not want to risk discouraging public discussion on this
> topic, but I am not sure if this is really worth it.

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

-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com