Re: [PATCH 1/5] Turn unpack_trees_options.msgs into an array + enum

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Aug 9, 2010, 20:49 UTC
Message-ID: <vpq8w4fse8w.fsf@bauges.imag.fr>
In-Reply-To: <7vwrrz4lgi.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 9 quoted lines

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> The list of error messages was introduced as a structure, but an array
>> indexed over an enum is more flexible, since it allows one to store a
>> type of error message (index in the array) in a variable.
>
> Hmm, the only example of the advantage "enum used as array index" offers
> that I can think of off the top of my head is that you can more easily
> iterate over it.

The portion of my message you cite give another: the ability to store the index easily in a variable (or to pass it as argument to a function). With a struct, you can easily pass field name to a macro, but not to a function.

The other is that you can have multiple arrays indexed over the same enum, which is what we do later: we'll have to manage an array of list of rejected paths, with one list per error kind.

(we'll also have to iterate over this second array)

The previous version of the patch serie was using both a struct and an enum + array, and I wanted to get rid of that.

> If you are to change them to enum, I would actually suggest renaming them
> a bit more to make them stand out.  Perhaps spell them all in caps,
> perhaps have them share the same short prefix (UTEM_ - unpack trees error
> messages), etc.

I'd say just "ERROR_*" (UTEM seems hard to read, and won't be used often enough for people to remember it), but I'm fine with all caps + prefix, yes. A quick grep shows this is how the rest of Git does.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/