[PATCH] CodingGuidelines: discourage arbitrary suffixes in function names

[PATCH] CodingGuidelines: discourage arbitrary suffixes in function names

[Karthik Nayak]

We often name functions with arbitrary suffixes like `_1` as an
extension of another existing function. This created 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>
---

This is mostly in response to an ongoing thread [1] where I ran into one of
these functions and it really took me a while to wrap my head around what the
function does.

[1]: https://lore.kernel.org/git/CAOLa=ZREg3xuaT6mbM8+Havn3regZDhK45kGy0+Fw8t56c7Mpg@mail.gmail.com/#R

 Documentation/CodingGuidelines | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 30fda4142c..b8e2d30567 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -621,6 +621,11 @@ 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. 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.
-- 
2.47.0

Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names

[Patrick Steinhardt]

On Mon, Oct 21, 2024 at 02:41:45PM +0200, Karthik Nayak wrote:
> We often name functions with arbitrary suffixes like `_1` as an
> extension of another existing function. This created confusion and

Micro-nit: s/created/creates/

[snip]
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index 30fda4142c..b8e2d30567 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -621,6 +621,11 @@ 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. To maintain clarity and avoid confusion,
> +   arbitrary suffixes such as _1 are discouraged, as they provide no
> +   meaningful insight into the function's role.

Names being self-explanatory is certainly a worthwhile goal, even though
I guess that it's a bit more on the idealized side of things. Functions
will often not be fully self-explanatory, which is likely just a matter
of reality. I mostly just don't want us to end on the other side of the
spectrum where we go militant on "Every function must be no longer than
2 lines of code such that it can be self-explanatory".

And yes, I'm of course stretching what you are saying quite a bit, I
know that this is not what you want to say. I'm merely writing down my
own thoughts while thinking it through.

So, that being said, I agree that we shouldn't use arbitrary suffixes,
as these are quite hard to understand indeed and typically don't really
provide any context. So as long as we interpret this rule leniently I'm
happy with it.

Patrick

Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names

[Kristoffer Haugsbakk]

On Mon, Oct 21, 2024, at 14:41, Karthik Nayak wrote:
> We often name functions with arbitrary suffixes like `_1` as an
> extension of another existing function. This created 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>
> ---
>
> This is mostly in response to an ongoing thread [1] where I ran into one of
> these functions and it really took me a while to wrap my head around what the
> function does.
>
> [1]:
> https://lore.kernel.org/git/CAOLa=ZREg3xuaT6mbM8+Havn3regZDhK45kGy0+Fw8t56c7Mpg@mail.gmail.com/#R

I was wondering whether it would make sense to use that link in this
document for the context.  But I see that there is only one instance
of that in the current document.

Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names

[Taylor Blau]

On Mon, Oct 21, 2024 at 02:59:00PM +0200, Patrick Steinhardt wrote:
> On Mon, Oct 21, 2024 at 02:41:45PM +0200, Karthik Nayak wrote:
> > We often name functions with arbitrary suffixes like `_1` as an
> > extension of another existing function. This created confusion and
>
> Micro-nit: s/created/creates/
>
> [snip]
> > diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> > index 30fda4142c..b8e2d30567 100644
> > --- a/Documentation/CodingGuidelines
> > +++ b/Documentation/CodingGuidelines
> > @@ -621,6 +621,11 @@ 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. To maintain clarity and avoid confusion,
> > +   arbitrary suffixes such as _1 are discouraged, as they provide no
> > +   meaningful insight into the function's role.
>
> Names being self-explanatory is certainly a worthwhile goal, even though
> I guess that it's a bit more on the idealized side of things. Functions
> will often not be fully self-explanatory, which is likely just a matter
> of reality. I mostly just don't want us to end on the other side of the
> spectrum where we go militant on "Every function must be no longer than
> 2 lines of code such that it can be self-explanatory".
>
> And yes, I'm of course stretching what you are saying quite a bit, I
> know that this is not what you want to say. I'm merely writing down my
> own thoughts while thinking it through.
>
> So, that being said, I agree that we shouldn't use arbitrary suffixes,
> as these are quite hard to understand indeed and typically don't really
> provide any context. So as long as we interpret this rule leniently I'm
> happy with it.

I am not sure here... I think that using a "_1()" suffix means that the
function is processing one of a number of elements that all need to be
handled similarly, but where both the processing of an individual
element, and the handling of a group of elements are both complicated
enough to be placed in their own function.

It's also a helpful convention when you have a recursive function that
does some setup during its initial call, but subsequent layers of
recursion don't need or want to repeat that setup.

So I'm not sure I agree that "_1()" is always a bad idea as this changes
suggests (i.e. by writing that "they provide no meaningful insight into
the function's role").

Perhaps we could rephrase what is written here to suggest a couple of
instances where we wouldn't want to apply this rule, and the two that I
have written above could perhaps be a useful starting point. But I lean
more towards not adjusting our CodingGuidelines at all here.

Thanks,
Taylor

Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names

[karthik nayak]

[-- Attachment #1: Type: text/plain, Size: 1995 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> On Mon, Oct 21, 2024 at 02:41:45PM +0200, Karthik Nayak wrote:
>> We often name functions with arbitrary suffixes like `_1` as an
>> extension of another existing function. This created confusion and
>
> Micro-nit: s/created/creates/
>
> [snip]
>> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
>> index 30fda4142c..b8e2d30567 100644
>> --- a/Documentation/CodingGuidelines
>> +++ b/Documentation/CodingGuidelines
>> @@ -621,6 +621,11 @@ 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. To maintain clarity and avoid confusion,
>> +   arbitrary suffixes such as _1 are discouraged, as they provide no
>> +   meaningful insight into the function's role.
>
> Names being self-explanatory is certainly a worthwhile goal, even though
> I guess that it's a bit more on the idealized side of things. Functions
> will often not be fully self-explanatory, which is likely just a matter
> of reality. I mostly just don't want us to end on the other side of the
> spectrum where we go militant on "Every function must be no longer than
> 2 lines of code such that it can be self-explanatory".
>
> And yes, I'm of course stretching what you are saying quite a bit, I
> know that this is not what you want to say. I'm merely writing down my
> own thoughts while thinking it through.
>

I agree, going the other way doesn't help either.

> So, that being said, I agree that we shouldn't use arbitrary suffixes,
> as these are quite hard to understand indeed and typically don't really
> provide any context. So as long as we interpret this rule leniently I'm
> happy with it.
>
> Patrick

I tried to keep the wording to also not just say "it is not allowed to
use such suffixes", but mostly to discourage it. I guess we can also
iterate on this as needed with time.

Karthik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names

[karthik nayak]

[-- Attachment #1: Type: text/plain, Size: 4233 bytes --]

Taylor Blau <me@ttaylorr.com> writes:

> On Mon, Oct 21, 2024 at 02:59:00PM +0200, Patrick Steinhardt wrote:
>> On Mon, Oct 21, 2024 at 02:41:45PM +0200, Karthik Nayak wrote:
>> > We often name functions with arbitrary suffixes like `_1` as an
>> > extension of another existing function. This created confusion and
>>
>> Micro-nit: s/created/creates/
>>
>> [snip]
>> > diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
>> > index 30fda4142c..b8e2d30567 100644
>> > --- a/Documentation/CodingGuidelines
>> > +++ b/Documentation/CodingGuidelines
>> > @@ -621,6 +621,11 @@ 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. To maintain clarity and avoid confusion,
>> > +   arbitrary suffixes such as _1 are discouraged, as they provide no
>> > +   meaningful insight into the function's role.
>>
>> Names being self-explanatory is certainly a worthwhile goal, even though
>> I guess that it's a bit more on the idealized side of things. Functions
>> will often not be fully self-explanatory, which is likely just a matter
>> of reality. I mostly just don't want us to end on the other side of the
>> spectrum where we go militant on "Every function must be no longer than
>> 2 lines of code such that it can be self-explanatory".
>>
>> And yes, I'm of course stretching what you are saying quite a bit, I
>> know that this is not what you want to say. I'm merely writing down my
>> own thoughts while thinking it through.
>>
>> So, that being said, I agree that we shouldn't use arbitrary suffixes,
>> as these are quite hard to understand indeed and typically don't really
>> provide any context. So as long as we interpret this rule leniently I'm
>> happy with it.
>
> I am not sure here... I think that using a "_1()" suffix means that the
> function is processing one of a number of elements that all need to be
> handled similarly, but where both the processing of an individual
> element, and the handling of a group of elements are both complicated
> enough to be placed in their own function.
>

The crux here is that this meaning is internalized by people who know
the code through in and out. But for anyone new to the project, this is
not evident from the naming.

> It's also a helpful convention when you have a recursive function that
> does some setup during its initial call, but subsequent layers of
> recursion don't need or want to repeat that setup.
>

I get the usecase of having such functions. I totally see the need, it's
mostly the naming that I'm trying to change.

Let's consider two of such functions:

1. mark_remote_island_1: This function is called to do _some_ work on a
single remote island when iterating over a list.
2. find_longest_prefixes_1: This is a recursive function which is used
to find the longest prefix.

If you notice, both use the "_1" suffix and do different things (operate
on a single instance from a list vs provide recursive behavior). So the
user has to read the code to understand, which makes the "_1" a bit
confusing.

Second, this could have instead been named:
1. mark_single_remote_island: Which reads better, giving the idea that
we are really working on a single remote island. Whereas having a "_1"
doesn't easily imply the same.
2. find_longest_prefixes_recursively: Which also reads better, and gives
a hint on how the function operates and why it is split out from the
base function.

> So I'm not sure I agree that "_1()" is always a bad idea as this changes
> suggests (i.e. by writing that "they provide no meaningful insight into
> the function's role").
>
> Perhaps we could rephrase what is written here to suggest a couple of
> instances where we wouldn't want to apply this rule, and the two that I
> have written above could perhaps be a useful starting point. But I lean
> more towards not adjusting our CodingGuidelines at all here.
>

I hope my reasoning convinces you of the benefits, this is mostly coming
from me spending more time than I'd like to admit on one of these
functions, trying to understand what they do :D

> Thanks,
> Taylor

Thanks,
Karthik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names

[karthik nayak]

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:

> On Mon, Oct 21, 2024, at 14:41, Karthik Nayak wrote:
>> We often name functions with arbitrary suffixes like `_1` as an
>> extension of another existing function. This created 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>
>> ---
>>
>> This is mostly in response to an ongoing thread [1] where I ran into one of
>> these functions and it really took me a while to wrap my head around what the
>> function does.
>>
>> [1]:
>> https://lore.kernel.org/git/CAOLa=ZREg3xuaT6mbM8+Havn3regZDhK45kGy0+Fw8t56c7Mpg@mail.gmail.com/#R
>
> I was wondering whether it would make sense to use that link in this
> document for the context.  But I see that there is only one instance
> of that in the current document.

Yeah, there are a few such functions in our codebase. I'd be happy to
make any improvements, but also think this is simple and clear at the
moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names

[Taylor Blau]

On Tue, Oct 22, 2024 at 03:45:38AM -0500, karthik nayak wrote:
> >> So, that being said, I agree that we shouldn't use arbitrary suffixes,
> >> as these are quite hard to understand indeed and typically don't really
> >> provide any context. So as long as we interpret this rule leniently I'm
> >> happy with it.
> >
> > I am not sure here... I think that using a "_1()" suffix means that the
> > function is processing one of a number of elements that all need to be
> > handled similarly, but where both the processing of an individual
> > element, and the handling of a group of elements are both complicated
> > enough to be placed in their own function.
>
> The crux here is that this meaning is internalized by people who know
> the code through in and out. But for anyone new to the project, this is
> not evident from the naming.

Right. I think that with that in mind, a good goal might be to document
that convention to make sure that newcomers to the project can easily
interpret what foo() and foo_1() mean. Another approach is the one you
pursue here, which is to change the existing convention in the name of
making the code more approachable for newcomers.

Both approaches meet the same end, but the former does not involve
changing existing conventions, but instead documenting them. That seems
like a more reasonable path to me.

> > It's also a helpful convention when you have a recursive function that
> > does some setup during its initial call, but subsequent layers of
> > recursion don't need or want to repeat that setup.
> >
>
> I get the usecase of having such functions. I totally see the need, it's
> mostly the naming that I'm trying to change.
>
> Let's consider two of such functions:
>
> 1. mark_remote_island_1: This function is called to do _some_ work on a
> single remote island when iterating over a list.
> 2. find_longest_prefixes_1: This is a recursive function which is used
> to find the longest prefix.
>
> If you notice, both use the "_1" suffix and do different things (operate
> on a single instance from a list vs provide recursive behavior). So the
> user has to read the code to understand, which makes the "_1" a bit
> confusing.
>
> Second, this could have instead been named:
> 1. mark_single_remote_island: Which reads better, giving the idea that
> we are really working on a single remote island. Whereas having a "_1"
> doesn't easily imply the same.
> 2. find_longest_prefixes_recursively: Which also reads better, and gives
> a hint on how the function operates and why it is split out from the
> base function.

I don't disagree that writing "single" or "recursively" can be
considered clearer. I think that the convention to suffix such functions
with "_1()" is more terse, but saves characters and can avoid awkward
line wrapping.

Thanks,
Taylor

Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names

[karthik nayak]

[-- Attachment #1: Type: text/plain, Size: 3244 bytes --]

Taylor Blau <me@ttaylorr.com> writes:

> On Tue, Oct 22, 2024 at 03:45:38AM -0500, karthik nayak wrote:
>> >> So, that being said, I agree that we shouldn't use arbitrary suffixes,
>> >> as these are quite hard to understand indeed and typically don't really
>> >> provide any context. So as long as we interpret this rule leniently I'm
>> >> happy with it.
>> >
>> > I am not sure here... I think that using a "_1()" suffix means that the
>> > function is processing one of a number of elements that all need to be
>> > handled similarly, but where both the processing of an individual
>> > element, and the handling of a group of elements are both complicated
>> > enough to be placed in their own function.
>>
>> The crux here is that this meaning is internalized by people who know
>> the code through in and out. But for anyone new to the project, this is
>> not evident from the naming.
>
> Right. I think that with that in mind, a good goal might be to document
> that convention to make sure that newcomers to the project can easily
> interpret what foo() and foo_1() mean. Another approach is the one you
> pursue here, which is to change the existing convention in the name of
> making the code more approachable for newcomers.
>
> Both approaches meet the same end, but the former does not involve
> changing existing conventions, but instead documenting them. That seems
> like a more reasonable path to me.
>

I would disagree though. I think some conventions even though we've been
using them for a while, should be changed. I guess a good middle ground
is to document current behavior while also discouraging future use. I'll
add that in and push a new version.

>> > It's also a helpful convention when you have a recursive function that
>> > does some setup during its initial call, but subsequent layers of
>> > recursion don't need or want to repeat that setup.
>> >
>>
>> I get the usecase of having such functions. I totally see the need, it's
>> mostly the naming that I'm trying to change.
>>
>> Let's consider two of such functions:
>>
>> 1. mark_remote_island_1: This function is called to do _some_ work on a
>> single remote island when iterating over a list.
>> 2. find_longest_prefixes_1: This is a recursive function which is used
>> to find the longest prefix.
>>
>> If you notice, both use the "_1" suffix and do different things (operate
>> on a single instance from a list vs provide recursive behavior). So the
>> user has to read the code to understand, which makes the "_1" a bit
>> confusing.
>>
>> Second, this could have instead been named:
>> 1. mark_single_remote_island: Which reads better, giving the idea that
>> we are really working on a single remote island. Whereas having a "_1"
>> doesn't easily imply the same.
>> 2. find_longest_prefixes_recursively: Which also reads better, and gives
>> a hint on how the function operates and why it is split out from the
>> base function.
>
> I don't disagree that writing "single" or "recursively" can be
> considered clearer. I think that the convention to suffix such functions
> with "_1()" is more terse, but saves characters and can avoid awkward
> line wrapping.
>

But are those pros worth the ambiguity it brings?

> Thanks,
> Taylor

- Karthik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v2] CodingGuidelines: discourage arbitrary suffixes in function names

[Karthik Nayak]

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(+)

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.

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.
     +
-- 
2.47.0

Re: [PATCH v2] CodingGuidelines: discourage arbitrary suffixes in function names

[Taylor Blau]

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

Re: [PATCH v2] CodingGuidelines: discourage arbitrary suffixes in function names

[Karthik Nayak]

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

Re: [PATCH v2] CodingGuidelines: discourage arbitrary suffixes in function names

[Justin Tobler]

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

Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> I don't disagree that writing "single" or "recursively" can be
> considered clearer. I think that the convention to suffix such functions
> with "_1()" is more terse, but saves characters and can avoid awkward
> line wrapping.

I am reasonably sure that I was the first user of the _1()
convention, or at least I was one of them.  The reason for the
choice of suffix was only because there wasn't anything suitable
when refactoring an existing function foo() into a set-up part and
its recursive body, so I just kept the set-up part and the single
call into the new function in the original foo(), and had to give a
name to the new function that holds the body of the original logic
that was moved from foo().

Neither foo_helper() or foo_recursive() were descriptive enough to
warrant such longer suffixes than a simple _1().  They easily can
get "help by doing what?" and "recursively doing what?" reaction,
which is a sure sign that the suffixes are not descriptive enough.

That was the only reason why I picked that "short-and-sweet but
cryptic" suffix.

Surely all of _1(), _helper(), _recursive() are meaningless.  If we
were to replace existing uses of them, the replacement has to be 10x
better.

Having said all that, as an aspirational goal, I think it is good to
encourage people to find a name that is descriptive when writing a
new function.  I'd refrain from judging if it is way too obvious to
be worth documenting (as I am officially on vacation and shouldn't
be thinking too much about the project).

Thanks.

Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names

[Taylor Blau]

On Wed, Oct 23, 2024 at 05:50:17PM -0700, Junio C Hamano wrote:
> Surely all of _1(), _helper(), _recursive() are meaningless.  If we
> were to replace existing uses of them, the replacement has to be 10x
> better.

Well put. Each of the three are more or less equally meaningless, but
_1() is an accepted (?) project convention and has the fewest
characters, so I think is a good choice personally.

> Having said all that, as an aspirational goal, I think it is good to
> encourage people to find a name that is descriptive when writing a
> new function.  I'd refrain from judging if it is way too obvious to
> be worth documenting (as I am officially on vacation and shouldn't
> be thinking too much about the project).

Yeah, go back to vacation ;-).

Thanks,
Taylor