diff mbox series

[v2] CodingGuidelines: discourage arbitrary suffixes in function names

Message ID 20241023075706.1393147-1-karthik.188@gmail.com (mailing list archive)
State Superseded
Headers show
Series [v2] CodingGuidelines: discourage arbitrary suffixes in function names | expand

Commit Message

Karthik Nayak Oct. 23, 2024, 7:57 a.m. UTC 
We often name functions with arbitrary suffixes like `_1` as an
extension of another existing function. This creates confusion and
doesn't provide good clarity into the functions purpose. Let's document
good function naming etiquette in our CodingGuidelines.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/CodingGuidelines | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)


Range-diff against v1:
1:  0acdf6902c ! 1:  dd556a8029 CodingGuidelines: discourage arbitrary suffixes in function names
    @@ Commit message
         CodingGuidelines: discourage arbitrary suffixes in function names
     
         We often name functions with arbitrary suffixes like `_1` as an
    -    extension of another existing function. This created confusion and
    +    extension of another existing function. This creates confusion and
         doesn't provide good clarity into the functions purpose. Let's document
         good function naming etiquette in our CodingGuidelines.
     
    @@ Documentation/CodingGuidelines: For C programs:
            structure.
      
     + - Function names should be self-explanatory, clearly reflecting their
    -+   purpose or behavior. To maintain clarity and avoid confusion,
    ++   purpose or behavior.
    ++
    ++   The '_1' suffix for function names has historically indicated:
    ++
    ++    - functions processing one of several elements that all need to be
    ++      handled similarly.
    ++
    ++    - recursive functions that need to be separated from a setup stage.
    ++
    ++   To maintain clarity and avoid confusion, such arbitrary suffixes are
    ++   discouraged, as they provide no meaningful insight into the function's
    ++   role.
    ++
    ++To maintain clarity and avoid confusion,
     +   arbitrary suffixes such as _1 are discouraged, as they provide no
     +   meaningful insight into the function's role.
     +

Comments

Taylor Blau Oct. 23, 2024, 8:34 p.m. UTC | #1 
On Wed, Oct 23, 2024 at 09:57:06AM +0200, Karthik Nayak wrote:
> + - Function names should be self-explanatory, clearly reflecting their
> +   purpose or behavior.
> +
> +   The '_1' suffix for function names has historically indicated:
> +
> +    - functions processing one of several elements that all need to be
> +      handled similarly.
> +
> +    - recursive functions that need to be separated from a setup stage.
> +
> +   To maintain clarity and avoid confusion, such arbitrary suffixes are
> +   discouraged, as they provide no meaningful insight into the function's
> +   role.
> +

I'm still not sold on the suggestion to discourage the use of '_1' in
the future, so we may want to further qualify this statement with cases
where it is OK (in the spirit of Patrick's "as long as this is loosely
applied" comment from earlier).

> +To maintain clarity and avoid confusion,
> +   arbitrary suffixes such as _1 are discouraged, as they provide no
> +   meaningful insight into the function's role.
> +

Stray diff from the first round?

Thanks,
Taylor
Karthik Nayak Oct. 23, 2024, 9:03 p.m. UTC | #2 
On Wed, Oct 23, 2024 at 10:34 PM Taylor Blau <me@ttaylorr.com> wrote:
>
> On Wed, Oct 23, 2024 at 09:57:06AM +0200, Karthik Nayak wrote:
> > + - Function names should be self-explanatory, clearly reflecting their
> > +   purpose or behavior.
> > +
> > +   The '_1' suffix for function names has historically indicated:
> > +
> > +    - functions processing one of several elements that all need to be
> > +      handled similarly.
> > +
> > +    - recursive functions that need to be separated from a setup stage.
> > +
> > +   To maintain clarity and avoid confusion, such arbitrary suffixes are
> > +   discouraged, as they provide no meaningful insight into the function's
> > +   role.
> > +
>
> I'm still not sold on the suggestion to discourage the use of '_1' in
> the future, so we may want to further qualify this statement with cases
> where it is OK (in the spirit of Patrick's "as long as this is loosely
> applied" comment from earlier).
>

I would say that in some sense goes against my motive for this patch,
and I still firmly stand by not having '_1' as a suffix. As such, I understand
that this might be my biased opinion, so I'll drop the patch overall since
like you mentioned, it probably is better to not have anything at all added
to the documentation.

> > +To maintain clarity and avoid confusion,
> > +   arbitrary suffixes such as _1 are discouraged, as they provide no
> > +   meaningful insight into the function's role.
> > +
>
> Stray diff from the first round?
>

Indeed :)

> Thanks,
> Taylor

Karthik
Justin Tobler Oct. 23, 2024, 11:07 p.m. UTC | #3 
On 24/10/23 04:34PM, Taylor Blau wrote:
> On Wed, Oct 23, 2024 at 09:57:06AM +0200, Karthik Nayak wrote:
> > + - Function names should be self-explanatory, clearly reflecting their
> > +   purpose or behavior.
> > +
> > +   The '_1' suffix for function names has historically indicated:
> > +
> > +    - functions processing one of several elements that all need to be
> > +      handled similarly.
> > +
> > +    - recursive functions that need to be separated from a setup stage.
> > +
> > +   To maintain clarity and avoid confusion, such arbitrary suffixes are
> > +   discouraged, as they provide no meaningful insight into the function's
> > +   role.
> > +
> 
> I'm still not sold on the suggestion to discourage the use of '_1' in
> the future, so we may want to further qualify this statement with cases
> where it is OK (in the spirit of Patrick's "as long as this is loosely
> applied" comment from earlier).

When I fist encountered the use of the '_1' suffix, I was also not
immediately sure of the intent it was trying to communicate. If I
remember correctly, there are also existing uses that don't really fit
into the two examples mentioned above. One such use I found is
`handle_revision_arg_1()`[1] where it seems we are just more generally
factoring out a subset of the parent functions logic, but keeping the
function name mostly the same. If this is also a usecase, maybe we
should instead say something more generic along the lines of:

"A function suffixed with '_1' indicates that a subset of logic from its
corresponding parent function has been factored out and encapsulated in
a separate function."

Regarding whether its use should be discouraged, if the convention is
well understood and documented it probably isn't much of an issue, but
for newcomers it may be another source of confusion as its meaning is
not implicitly well understood. I do think its a good first step to
improve the documentation here though. :)

-Justin

[1] https://gitlab.com/gitlab-org/git/-/blob/fd3785337beb285ed7fd67ce6fc3d3bed2097b40/revision.c#L2176
diff mbox series

Patch

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 30fda4142c..635d6f3a27 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -621,6 +621,24 @@  For C programs:
     - `S_free()` releases a structure's contents and frees the
       structure.
 
+ - Function names should be self-explanatory, clearly reflecting their
+   purpose or behavior.
+
+   The '_1' suffix for function names has historically indicated:
+
+    - functions processing one of several elements that all need to be
+      handled similarly.
+
+    - recursive functions that need to be separated from a setup stage.
+
+   To maintain clarity and avoid confusion, such arbitrary suffixes are
+   discouraged, as they provide no meaningful insight into the function's
+   role.
+
+To maintain clarity and avoid confusion,
+   arbitrary suffixes such as _1 are discouraged, as they provide no
+   meaningful insight into the function's role.
+
 For Perl programs:
 
  - Most of the C guidelines above apply.