Re: [PATCH 2/2] dir: improve naming of oid_stat fields in two structs

From: Junio C Hamano <gitster@pobox.com>
To: Matheus Tavares <matheus.bernardino@usp.br>
Cc: git@vger.kernel.org, "René Scharfe" <l.s.r@web.de>,
	"Jeff King" <peff@peff.net>
Subject: Re: [PATCH 2/2] dir: improve naming of oid_stat fields in two structs
Date: Sun, 15 Mar 2020 23:17:02 -0700	[thread overview]
Message-ID: <xmqq1rpsako1.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <6fee28469e49d501e5184162bc820350f60cc3de.1584329834.git.matheus.bernardino@usp.br> (Matheus Tavares's message of "Mon, 16 Mar 2020 00:47:27 -0300")

Matheus Tavares <matheus.bernardino@usp.br> writes:

> Note: I choosed to use "st_*", as naming, for simplicity, and to keep
> the code lines small. But should we use a more verbose "oidst_*" format,
> instead, to avoid confusions with "struct stat"?
> ...
> @@ -334,8 +334,8 @@ struct dir_struct {
>  
>  	/* Enable untracked file cache if set */
>  	struct untracked_cache *untracked;
> -	struct oid_stat ss_info_exclude;
> -	struct oid_stat ss_excludes_file;
> +	struct oid_stat st_info_exclude;
> +	struct oid_stat st_excludes_file;
>  	unsigned unmanaged_exclude_files;
>  };

I tend to agree with you that using prefix "st_" for anything other
than "struct stat" proper would be confusing.  If "ss" used to stand
for "sha1 stat", I would expect "oid stat" to abbreviate to "os", at
least.

I also am wondering if we can do without any prefix, i.e. just call
them "info_exclude" and "excludes_file", because the field names are
private to each struct and there is no strong reason to have a
common prefix among fields in a single struct.  Rather, it is more
important for the fields of the same type in a single struct to have
distinct names so that the reader can easily tell them apart and the
reason for their use is straight-forward to understand, and the two
names without any prefix convey their distinction pretty well, I
would think.

It is not like we adopt a convention to name our variables with
abbreviated typenames embedded in the variable names.  We shouldn't
be doing that for field names, either, but it smells that the "give
them prefix ss_ because they are of type sha1_stat" pretty much has
origin in such school.

Thanks.