git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: karthik nayak <karthik.188@gmail.com>
Cc: Patrick Steinhardt <ps@pks.im>, git@vger.kernel.org
Subject: Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names
Date: Tue, 22 Oct 2024 12:41:37 -0400	[thread overview]
Message-ID: <ZxfVwQxMlcJbGt5D@nand.local> (raw)
In-Reply-To: <CAOLa=ZSPmkdngn3=cksBM-syO94-zYANLk8FWBsQYpTR8XT9FA@mail.gmail.com>

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

  reply	other threads:[~2024-10-22 16:41 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-21 12:41 [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names Karthik Nayak
2024-10-21 12:59 ` Patrick Steinhardt
2024-10-21 20:02   ` Taylor Blau
2024-10-22  8:45     ` karthik nayak
2024-10-22 16:41       ` Taylor Blau [this message]
2024-10-23  7:44         ` karthik nayak
2024-10-24  0:50         ` Junio C Hamano
2024-10-24 16:50           ` Taylor Blau
2024-10-22  8:34   ` karthik nayak
2024-10-21 16:51 ` Kristoffer Haugsbakk
2024-10-22  8:47   ` karthik nayak
2024-10-23  7:57 ` [PATCH v2] " Karthik Nayak
2024-10-23 20:34   ` Taylor Blau
2024-10-23 21:03     ` Karthik Nayak
2024-10-23 23:07     ` Justin Tobler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZxfVwQxMlcJbGt5D@nand.local \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=karthik.188@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).