Re: [PATCH 05/15] get_ref_map(): rename local variables

[Michael Haggerty]

On 10/23/2013 08:45 PM, Junio C Hamano wrote:
> Michael Haggerty  writes:
> 
>> Rename "refs" -> "refspecs" and "ref_count" -> "refspec_count" to
>> reduce confusion, because they describe an array of "struct refspec",
>> as opposed to the "struct ref" objects that are also used in this
>> function.
> 
> Good.  In general, we'd prefer to name an array of things that are
> primarily walked in the index order "thing[]", so that "thing number
> 3" can be spelled thing[3] (not things[3]) in the code, though.

Since I didn't change singular -> plural or vice versa in this patch,
it's a bit off topic, but in case you are curious I prefer plural to
distinguish which pointers point at lists or arrays as opposed to single
objects.  This convention conveniently leaves the singular available to
name a variable that is used for a single object; for example, in a loop

struct thing thing = things[i];

(The convention in SQL is different: tables are usually named using
singular nouns.  But that makes sense in SQL because there is not really
a way to reference a single row in a table as an aggregate, so there is
no need to reserve the singular noun for that purpose.  In fact, in
SELECT statements the table name often appears in a context that makes
it look like it does refer to a single row:

SELECT employee.name, employee.salary FROM ...

So I think it makes sense to use different conventions in C vs. SQL.)

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/15] get_ref_map(): rename local variables

[Junio C Hamano]

Michael Haggerty  writes:

> Rename "refs" -> "refspecs" and "ref_count" -> "refspec_count" to
> reduce confusion, because they describe an array of "struct refspec",
> as opposed to the "struct ref" objects that are also used in this
> function.

Good.  In general, we'd prefer to name an array of things that are
primarily walked in the index order "thing[]", so that "thing number
3" can be spelled thing[3] (not things[3]) in the code, though.

> Signed-off-by: Michael Haggerty 
> ---
>  builtin/fetch.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index bd7a101..2248abf 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -165,8 +165,8 @@ static void find_non_local_tags(struct transport 
> *transport,
>   struct ref ***tail);
>  
>  static struct ref *get_ref_map(struct transport *transport,
> -struct refspec *refs, int ref_count, int tags,
> -int *autotags)
> +struct refspec *refspecs, int refspec_count,
> +int tags, int *autotags)
>  {
>   int i;
>   struct ref *rm;
> @@ -175,12 +175,12 @@ static struct ref *get_ref_map(struct transport 
> *transport,
>  
>   const struct ref *remote_refs = transport_get_remote_refs(transport);
>  
> - if (ref_count || tags == TAGS_SET) {
> + if (refspec_count || tags == TAGS_SET) {
>   struct ref **old_tail;
>  
> - for (i = 0; i < ref_count; i++) {
> - get_fetch_map(remote_refs, &refs[i], &tail, 0);
> - if (refs[i].dst && refs[i].dst[0])
> + for (i = 0; i < refspec_count; i++) {
> + get_fetch_map(remote_refs, &refspecs[i], &tail, 0);
> + if (refspecs[i].dst && refspecs[i].dst[0])
>   *autotags = 1;
>   }
>   /* Merge everything on the command line, but not --tags */
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html