{"thread":{"id":"62382","subject":"[PATCH] CodingGuidelines: discourage arbitrary suffixes in function names","startedAt":"2024-10-21T12:41:49Z","lastAt":"2024-10-24T16:50:07Z","messageCount":15,"participants":["Karthik Nayak","Patrick Steinhardt","Kristoffer Haugsbakk","Taylor Blau","karthik nayak","Justin Tobler","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"505674","messageId":"20241021124145.636561-1-karthik.188@gmail.com","threadId":"62382","inReplyTo":null,"subject":"[PATCH] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-10-21T12:41:45Z","receivedAt":"2024-10-21T12:41:49Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"We often name functions with arbitrary suffixes like `_1` as an\nextension of another existing function. This created confusion and\ndoesn't provide good clarity into the functions purpose. Let's document\ngood function naming etiquette in our CodingGuidelines.\n\nSigned-off-by: Karthik Nayak \n---\n\nThis is mostly in response to an ongoing thread [1] where I ran into one of\nthese functions and it really took me a while to wrap my head around what the\nfunction does.\n\n[1]: https://lore.kernel.org/git/CAOLa=ZREg3xuaT6mbM8+Havn3regZDhK45kGy0+Fw8t56c7Mpg@mail.gmail.com/#R\n\n Documentation/CodingGuidelines | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 30fda4142c..b8e2d30567 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -621,6 +621,11 @@ For C programs:\n - `S_free()` releases a structure's contents and frees the\n structure.\n \n+ - Function names should be self-explanatory, clearly reflecting their\n+ purpose or behavior. To maintain clarity and avoid confusion,\n+ arbitrary suffixes such as _1 are discouraged, as they provide no\n+ meaningful insight into the function's role.\n+\n For Perl programs:\n \n - Most of the C guidelines above apply.\n-- \n2.47.0\n\n"},{"id":"505676","messageId":"ZxZP01C-9RngukZy@pks.im","threadId":"62382","inReplyTo":"20241021124145.636561-1-karthik.188@gmail.com","subject":"Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T12:59:00Z","receivedAt":"2024-10-21T12:59:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 21, 2024 at 02:41:45PM +0200, Karthik Nayak wrote:\n> We often name functions with arbitrary suffixes like `_1` as an\n> extension of another existing function. This created confusion and\n\nMicro-nit: s/created/creates/\n\n[snip]\n> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n> index 30fda4142c..b8e2d30567 100644\n> --- a/Documentation/CodingGuidelines\n> +++ b/Documentation/CodingGuidelines\n> @@ -621,6 +621,11 @@ For C programs:\n> - `S_free()` releases a structure's contents and frees the\n> structure.\n> \n> + - Function names should be self-explanatory, clearly reflecting their\n> + purpose or behavior. To maintain clarity and avoid confusion,\n> + arbitrary suffixes such as _1 are discouraged, as they provide no\n> + meaningful insight into the function's role.\n\nNames being self-explanatory is certainly a worthwhile goal, even though\nI guess that it's a bit more on the idealized side of things. Functions\nwill often not be fully self-explanatory, which is likely just a matter\nof reality. I mostly just don't want us to end on the other side of the\nspectrum where we go militant on \"Every function must be no longer than\n2 lines of code such that it can be self-explanatory\".\n\nAnd yes, I'm of course stretching what you are saying quite a bit, I\nknow that this is not what you want to say. I'm merely writing down my\nown thoughts while thinking it through.\n\nSo, that being said, I agree that we shouldn't use arbitrary suffixes,\nas these are quite hard to understand indeed and typically don't really\nprovide any context. So as long as we interpret this rule leniently I'm\nhappy with it.\n\nPatrick\n"},{"id":"505712","messageId":"aba3d76c-5234-4875-a1a7-b212bb27b266@app.fastmail.com","threadId":"62382","inReplyTo":"20241021124145.636561-1-karthik.188@gmail.com","subject":"Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2024-10-21T16:51:22Z","receivedAt":"2024-10-21T16:51:44Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Mon, Oct 21, 2024, at 14:41, Karthik Nayak wrote:\n> We often name functions with arbitrary suffixes like `_1` as an\n> extension of another existing function. This created confusion and\n> doesn't provide good clarity into the functions purpose. Let's document\n> good function naming etiquette in our CodingGuidelines.\n>\n> Signed-off-by: Karthik Nayak \n> ---\n>\n> This is mostly in response to an ongoing thread [1] where I ran into one of\n> these functions and it really took me a while to wrap my head around what the\n> function does.\n>\n> [1]:\n> https://lore.kernel.org/git/CAOLa=ZREg3xuaT6mbM8+Havn3regZDhK45kGy0+Fw8t56c7Mpg@mail.gmail.com/#R\n\nI was wondering whether it would make sense to use that link in this\ndocument for the context. But I see that there is only one instance\nof that in the current document.\n"},{"id":"505739","messageId":"ZxazVG65+W0qKLXh@nand.local","threadId":"62382","inReplyTo":"ZxZP01C-9RngukZy@pks.im","subject":"Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-21T20:02:28Z","receivedAt":"2024-10-21T20:02:31Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 21, 2024 at 02:59:00PM +0200, Patrick Steinhardt wrote:\n> On Mon, Oct 21, 2024 at 02:41:45PM +0200, Karthik Nayak wrote:\n> > We often name functions with arbitrary suffixes like `_1` as an\n> > extension of another existing function. This created confusion and\n>\n> Micro-nit: s/created/creates/\n>\n> [snip]\n> > diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n> > index 30fda4142c..b8e2d30567 100644\n> > --- a/Documentation/CodingGuidelines\n> > +++ b/Documentation/CodingGuidelines\n> > @@ -621,6 +621,11 @@ For C programs:\n> > - `S_free()` releases a structure's contents and frees the\n> > structure.\n> >\n> > + - Function names should be self-explanatory, clearly reflecting their\n> > + purpose or behavior. To maintain clarity and avoid confusion,\n> > + arbitrary suffixes such as _1 are discouraged, as they provide no\n> > + meaningful insight into the function's role.\n>\n> Names being self-explanatory is certainly a worthwhile goal, even though\n> I guess that it's a bit more on the idealized side of things. Functions\n> will often not be fully self-explanatory, which is likely just a matter\n> of reality. I mostly just don't want us to end on the other side of the\n> spectrum where we go militant on \"Every function must be no longer than\n> 2 lines of code such that it can be self-explanatory\".\n>\n> And yes, I'm of course stretching what you are saying quite a bit, I\n> know that this is not what you want to say. I'm merely writing down my\n> own thoughts while thinking it through.\n>\n> So, that being said, I agree that we shouldn't use arbitrary suffixes,\n> as these are quite hard to understand indeed and typically don't really\n> provide any context. So as long as we interpret this rule leniently I'm\n> happy with it.\n\nI am not sure here... I think that using a \"_1()\" suffix means that the\nfunction is processing one of a number of elements that all need to be\nhandled similarly, but where both the processing of an individual\nelement, and the handling of a group of elements are both complicated\nenough to be placed in their own function.\n\nIt's also a helpful convention when you have a recursive function that\ndoes some setup during its initial call, but subsequent layers of\nrecursion don't need or want to repeat that setup.\n\nSo I'm not sure I agree that \"_1()\" is always a bad idea as this changes\nsuggests (i.e. by writing that \"they provide no meaningful insight into\nthe function's role\").\n\nPerhaps we could rephrase what is written here to suggest a couple of\ninstances where we wouldn't want to apply this rule, and the two that I\nhave written above could perhaps be a useful starting point. But I lean\nmore towards not adjusting our CodingGuidelines at all here.\n\nThanks,\nTaylor\n"},{"id":"505812","messageId":"CAOLa=ZSBg9Ji5g10M9vxC7UzMG=rHsNS31Z_obpznfAM7RHrXg@mail.gmail.com","threadId":"62382","inReplyTo":"ZxZP01C-9RngukZy@pks.im","subject":"Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-10-22T08:34:04Z","receivedAt":"2024-10-22T08:34:06Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Mon, Oct 21, 2024 at 02:41:45PM +0200, Karthik Nayak wrote:\n>> We often name functions with arbitrary suffixes like `_1` as an\n>> extension of another existing function. This created confusion and\n>\n> Micro-nit: s/created/creates/\n>\n> [snip]\n>> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n>> index 30fda4142c..b8e2d30567 100644\n>> --- a/Documentation/CodingGuidelines\n>> +++ b/Documentation/CodingGuidelines\n>> @@ -621,6 +621,11 @@ For C programs:\n>> - `S_free()` releases a structure's contents and frees the\n>> structure.\n>>\n>> + - Function names should be self-explanatory, clearly reflecting their\n>> + purpose or behavior. To maintain clarity and avoid confusion,\n>> + arbitrary suffixes such as _1 are discouraged, as they provide no\n>> + meaningful insight into the function's role.\n>\n> Names being self-explanatory is certainly a worthwhile goal, even though\n> I guess that it's a bit more on the idealized side of things. Functions\n> will often not be fully self-explanatory, which is likely just a matter\n> of reality. I mostly just don't want us to end on the other side of the\n> spectrum where we go militant on \"Every function must be no longer than\n> 2 lines of code such that it can be self-explanatory\".\n>\n> And yes, I'm of course stretching what you are saying quite a bit, I\n> know that this is not what you want to say. I'm merely writing down my\n> own thoughts while thinking it through.\n>\n\nI agree, going the other way doesn't help either.\n\n> So, that being said, I agree that we shouldn't use arbitrary suffixes,\n> as these are quite hard to understand indeed and typically don't really\n> provide any context. So as long as we interpret this rule leniently I'm\n> happy with it.\n>\n> Patrick\n\nI tried to keep the wording to also not just say \"it is not allowed to\nuse such suffixes\", but mostly to discourage it. I guess we can also\niterate on this as needed with time.\n\nKarthik\n"},{"id":"505813","messageId":"CAOLa=ZSPmkdngn3=cksBM-syO94-zYANLk8FWBsQYpTR8XT9FA@mail.gmail.com","threadId":"62382","inReplyTo":"ZxazVG65+W0qKLXh@nand.local","subject":"Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-10-22T08:45:38Z","receivedAt":"2024-10-22T08:45:40Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Taylor Blau writes:\n\n> On Mon, Oct 21, 2024 at 02:59:00PM +0200, Patrick Steinhardt wrote:\n>> On Mon, Oct 21, 2024 at 02:41:45PM +0200, Karthik Nayak wrote:\n>> > We often name functions with arbitrary suffixes like `_1` as an\n>> > extension of another existing function. This created confusion and\n>>\n>> Micro-nit: s/created/creates/\n>>\n>> [snip]\n>> > diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n>> > index 30fda4142c..b8e2d30567 100644\n>> > --- a/Documentation/CodingGuidelines\n>> > +++ b/Documentation/CodingGuidelines\n>> > @@ -621,6 +621,11 @@ For C programs:\n>> > - `S_free()` releases a structure's contents and frees the\n>> > structure.\n>> >\n>> > + - Function names should be self-explanatory, clearly reflecting their\n>> > + purpose or behavior. To maintain clarity and avoid confusion,\n>> > + arbitrary suffixes such as _1 are discouraged, as they provide no\n>> > + meaningful insight into the function's role.\n>>\n>> Names being self-explanatory is certainly a worthwhile goal, even though\n>> I guess that it's a bit more on the idealized side of things. Functions\n>> will often not be fully self-explanatory, which is likely just a matter\n>> of reality. I mostly just don't want us to end on the other side of the\n>> spectrum where we go militant on \"Every function must be no longer than\n>> 2 lines of code such that it can be self-explanatory\".\n>>\n>> And yes, I'm of course stretching what you are saying quite a bit, I\n>> know that this is not what you want to say. I'm merely writing down my\n>> own thoughts while thinking it through.\n>>\n>> So, that being said, I agree that we shouldn't use arbitrary suffixes,\n>> as these are quite hard to understand indeed and typically don't really\n>> provide any context. So as long as we interpret this rule leniently I'm\n>> happy with it.\n>\n> I am not sure here... I think that using a \"_1()\" suffix means that the\n> function is processing one of a number of elements that all need to be\n> handled similarly, but where both the processing of an individual\n> element, and the handling of a group of elements are both complicated\n> enough to be placed in their own function.\n>\n\nThe crux here is that this meaning is internalized by people who know\nthe code through in and out. But for anyone new to the project, this is\nnot evident from the naming.\n\n> It's also a helpful convention when you have a recursive function that\n> does some setup during its initial call, but subsequent layers of\n> recursion don't need or want to repeat that setup.\n>\n\nI get the usecase of having such functions. I totally see the need, it's\nmostly the naming that I'm trying to change.\n\nLet's consider two of such functions:\n\n1. mark_remote_island_1: This function is called to do _some_ work on a\nsingle remote island when iterating over a list.\n2. find_longest_prefixes_1: This is a recursive function which is used\nto find the longest prefix.\n\nIf you notice, both use the \"_1\" suffix and do different things (operate\non a single instance from a list vs provide recursive behavior). So the\nuser has to read the code to understand, which makes the \"_1\" a bit\nconfusing.\n\nSecond, this could have instead been named:\n1. mark_single_remote_island: Which reads better, giving the idea that\nwe are really working on a single remote island. Whereas having a \"_1\"\ndoesn't easily imply the same.\n2. find_longest_prefixes_recursively: Which also reads better, and gives\na hint on how the function operates and why it is split out from the\nbase function.\n\n> So I'm not sure I agree that \"_1()\" is always a bad idea as this changes\n> suggests (i.e. by writing that \"they provide no meaningful insight into\n> the function's role\").\n>\n> Perhaps we could rephrase what is written here to suggest a couple of\n> instances where we wouldn't want to apply this rule, and the two that I\n> have written above could perhaps be a useful starting point. But I lean\n> more towards not adjusting our CodingGuidelines at all here.\n>\n\nI hope my reasoning convinces you of the benefits, this is mostly coming\nfrom me spending more time than I'd like to admit on one of these\nfunctions, trying to understand what they do :D\n\n> Thanks,\n> Taylor\n\nThanks,\nKarthik\n"},{"id":"505814","messageId":"CAOLa=ZSWTKzgep+r8Qk9rZM_W28WsOS-k=VF=R2PHNMHLR613g@mail.gmail.com","threadId":"62382","inReplyTo":"aba3d76c-5234-4875-a1a7-b212bb27b266@app.fastmail.com","subject":"Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-10-22T08:47:19Z","receivedAt":"2024-10-22T08:47:21Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\"Kristoffer Haugsbakk\" writes:\n\n> On Mon, Oct 21, 2024, at 14:41, Karthik Nayak wrote:\n>> We often name functions with arbitrary suffixes like `_1` as an\n>> extension of another existing function. This created confusion and\n>> doesn't provide good clarity into the functions purpose. Let's document\n>> good function naming etiquette in our CodingGuidelines.\n>>\n>> Signed-off-by: Karthik Nayak \n>> ---\n>>\n>> This is mostly in response to an ongoing thread [1] where I ran into one of\n>> these functions and it really took me a while to wrap my head around what the\n>> function does.\n>>\n>> [1]:\n>> https://lore.kernel.org/git/CAOLa=ZREg3xuaT6mbM8+Havn3regZDhK45kGy0+Fw8t56c7Mpg@mail.gmail.com/#R\n>\n> I was wondering whether it would make sense to use that link in this\n> document for the context. But I see that there is only one instance\n> of that in the current document.\n\nYeah, there are a few such functions in our codebase. I'd be happy to\nmake any improvements, but also think this is simple and clear at the\nmoment.\n"},{"id":"505843","messageId":"ZxfVwQxMlcJbGt5D@nand.local","threadId":"62382","inReplyTo":"CAOLa=ZSPmkdngn3=cksBM-syO94-zYANLk8FWBsQYpTR8XT9FA@mail.gmail.com","subject":"Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-22T16:41:37Z","receivedAt":"2024-10-22T16:41:41Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Oct 22, 2024 at 03:45:38AM -0500, karthik nayak wrote:\n> >> So, that being said, I agree that we shouldn't use arbitrary suffixes,\n> >> as these are quite hard to understand indeed and typically don't really\n> >> provide any context. So as long as we interpret this rule leniently I'm\n> >> happy with it.\n> >\n> > I am not sure here... I think that using a \"_1()\" suffix means that the\n> > function is processing one of a number of elements that all need to be\n> > handled similarly, but where both the processing of an individual\n> > element, and the handling of a group of elements are both complicated\n> > enough to be placed in their own function.\n>\n> The crux here is that this meaning is internalized by people who know\n> the code through in and out. But for anyone new to the project, this is\n> not evident from the naming.\n\nRight. I think that with that in mind, a good goal might be to document\nthat convention to make sure that newcomers to the project can easily\ninterpret what foo() and foo_1() mean. Another approach is the one you\npursue here, which is to change the existing convention in the name of\nmaking the code more approachable for newcomers.\n\nBoth approaches meet the same end, but the former does not involve\nchanging existing conventions, but instead documenting them. That seems\nlike a more reasonable path to me.\n\n> > It's also a helpful convention when you have a recursive function that\n> > does some setup during its initial call, but subsequent layers of\n> > recursion don't need or want to repeat that setup.\n> >\n>\n> I get the usecase of having such functions. I totally see the need, it's\n> mostly the naming that I'm trying to change.\n>\n> Let's consider two of such functions:\n>\n> 1. mark_remote_island_1: This function is called to do _some_ work on a\n> single remote island when iterating over a list.\n> 2. find_longest_prefixes_1: This is a recursive function which is used\n> to find the longest prefix.\n>\n> If you notice, both use the \"_1\" suffix and do different things (operate\n> on a single instance from a list vs provide recursive behavior). So the\n> user has to read the code to understand, which makes the \"_1\" a bit\n> confusing.\n>\n> Second, this could have instead been named:\n> 1. mark_single_remote_island: Which reads better, giving the idea that\n> we are really working on a single remote island. Whereas having a \"_1\"\n> doesn't easily imply the same.\n> 2. find_longest_prefixes_recursively: Which also reads better, and gives\n> a hint on how the function operates and why it is split out from the\n> base function.\n\nI don't disagree that writing \"single\" or \"recursively\" can be\nconsidered clearer. I think that the convention to suffix such functions\nwith \"_1()\" is more terse, but saves characters and can avoid awkward\nline wrapping.\n\nThanks,\nTaylor\n"},{"id":"505904","messageId":"CAOLa=ZTSVmpDX-Os-0fs=-t4O5bN5H=aWSdxpMTSM1-UFFvfYQ@mail.gmail.com","threadId":"62382","inReplyTo":"ZxfVwQxMlcJbGt5D@nand.local","subject":"Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-10-23T07:44:00Z","receivedAt":"2024-10-23T07:44:02Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Taylor Blau writes:\n\n> On Tue, Oct 22, 2024 at 03:45:38AM -0500, karthik nayak wrote:\n>> >> So, that being said, I agree that we shouldn't use arbitrary suffixes,\n>> >> as these are quite hard to understand indeed and typically don't really\n>> >> provide any context. So as long as we interpret this rule leniently I'm\n>> >> happy with it.\n>> >\n>> > I am not sure here... I think that using a \"_1()\" suffix means that the\n>> > function is processing one of a number of elements that all need to be\n>> > handled similarly, but where both the processing of an individual\n>> > element, and the handling of a group of elements are both complicated\n>> > enough to be placed in their own function.\n>>\n>> The crux here is that this meaning is internalized by people who know\n>> the code through in and out. But for anyone new to the project, this is\n>> not evident from the naming.\n>\n> Right. I think that with that in mind, a good goal might be to document\n> that convention to make sure that newcomers to the project can easily\n> interpret what foo() and foo_1() mean. Another approach is the one you\n> pursue here, which is to change the existing convention in the name of\n> making the code more approachable for newcomers.\n>\n> Both approaches meet the same end, but the former does not involve\n> changing existing conventions, but instead documenting them. That seems\n> like a more reasonable path to me.\n>\n\nI would disagree though. I think some conventions even though we've been\nusing them for a while, should be changed. I guess a good middle ground\nis to document current behavior while also discouraging future use. I'll\nadd that in and push a new version.\n\n>> > It's also a helpful convention when you have a recursive function that\n>> > does some setup during its initial call, but subsequent layers of\n>> > recursion don't need or want to repeat that setup.\n>> >\n>>\n>> I get the usecase of having such functions. I totally see the need, it's\n>> mostly the naming that I'm trying to change.\n>>\n>> Let's consider two of such functions:\n>>\n>> 1. mark_remote_island_1: This function is called to do _some_ work on a\n>> single remote island when iterating over a list.\n>> 2. find_longest_prefixes_1: This is a recursive function which is used\n>> to find the longest prefix.\n>>\n>> If you notice, both use the \"_1\" suffix and do different things (operate\n>> on a single instance from a list vs provide recursive behavior). So the\n>> user has to read the code to understand, which makes the \"_1\" a bit\n>> confusing.\n>>\n>> Second, this could have instead been named:\n>> 1. mark_single_remote_island: Which reads better, giving the idea that\n>> we are really working on a single remote island. Whereas having a \"_1\"\n>> doesn't easily imply the same.\n>> 2. find_longest_prefixes_recursively: Which also reads better, and gives\n>> a hint on how the function operates and why it is split out from the\n>> base function.\n>\n> I don't disagree that writing \"single\" or \"recursively\" can be\n> considered clearer. I think that the convention to suffix such functions\n> with \"_1()\" is more terse, but saves characters and can avoid awkward\n> line wrapping.\n>\n\nBut are those pros worth the ambiguity it brings?\n\n> Thanks,\n> Taylor\n\n- Karthik\n"},{"id":"505905","messageId":"20241023075706.1393147-1-karthik.188@gmail.com","threadId":"62382","inReplyTo":"20241021124145.636561-1-karthik.188@gmail.com","subject":"[PATCH v2] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-10-23T07:57:06Z","receivedAt":"2024-10-23T07:57:12Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"We often name functions with arbitrary suffixes like `_1` as an\nextension of another existing function. This creates confusion and\ndoesn't provide good clarity into the functions purpose. Let's document\ngood function naming etiquette in our CodingGuidelines.\n\nSigned-off-by: Karthik Nayak \n---\n Documentation/CodingGuidelines | 18 ++++++++++++++++++\n 1 file changed, 18 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 30fda4142c..635d6f3a27 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -621,6 +621,24 @@ For C programs:\n - `S_free()` releases a structure's contents and frees the\n structure.\n \n+ - Function names should be self-explanatory, clearly reflecting their\n+ purpose or behavior.\n+\n+ The '_1' suffix for function names has historically indicated:\n+\n+ - functions processing one of several elements that all need to be\n+ handled similarly.\n+\n+ - recursive functions that need to be separated from a setup stage.\n+\n+ To maintain clarity and avoid confusion, such arbitrary suffixes are\n+ discouraged, as they provide no meaningful insight into the function's\n+ role.\n+\n+To maintain clarity and avoid confusion,\n+ arbitrary suffixes such as _1 are discouraged, as they provide no\n+ meaningful insight into the function's role.\n+\n For Perl programs:\n \n - Most of the C guidelines above apply.\n\nRange-diff against v1:\n1: 0acdf6902c ! 1: dd556a8029 CodingGuidelines: discourage arbitrary suffixes in function names\n @@ Commit message\n CodingGuidelines: discourage arbitrary suffixes in function names\n \n We often name functions with arbitrary suffixes like `_1` as an\n - extension of another existing function. This created confusion and\n + extension of another existing function. This creates confusion and\n doesn't provide good clarity into the functions purpose. Let's document\n good function naming etiquette in our CodingGuidelines.\n \n @@ Documentation/CodingGuidelines: For C programs:\n structure.\n \n + - Function names should be self-explanatory, clearly reflecting their\n -+ purpose or behavior. To maintain clarity and avoid confusion,\n ++ purpose or behavior.\n ++\n ++ The '_1' suffix for function names has historically indicated:\n ++\n ++ - functions processing one of several elements that all need to be\n ++ handled similarly.\n ++\n ++ - recursive functions that need to be separated from a setup stage.\n ++\n ++ To maintain clarity and avoid confusion, such arbitrary suffixes are\n ++ discouraged, as they provide no meaningful insight into the function's\n ++ role.\n ++\n ++To maintain clarity and avoid confusion,\n + arbitrary suffixes such as _1 are discouraged, as they provide no\n + meaningful insight into the function's role.\n +\n-- \n2.47.0\n\n"},{"id":"505981","messageId":"Zxld62svV9iDCcJJ@nand.local","threadId":"62382","inReplyTo":"20241023075706.1393147-1-karthik.188@gmail.com","subject":"Re: [PATCH v2] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-23T20:34:51Z","receivedAt":"2024-10-23T20:34:53Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Oct 23, 2024 at 09:57:06AM +0200, Karthik Nayak wrote:\n> + - Function names should be self-explanatory, clearly reflecting their\n> + purpose or behavior.\n> +\n> + The '_1' suffix for function names has historically indicated:\n> +\n> + - functions processing one of several elements that all need to be\n> + handled similarly.\n> +\n> + - recursive functions that need to be separated from a setup stage.\n> +\n> + To maintain clarity and avoid confusion, such arbitrary suffixes are\n> + discouraged, as they provide no meaningful insight into the function's\n> + role.\n> +\n\nI'm still not sold on the suggestion to discourage the use of '_1' in\nthe future, so we may want to further qualify this statement with cases\nwhere it is OK (in the spirit of Patrick's \"as long as this is loosely\napplied\" comment from earlier).\n\n> +To maintain clarity and avoid confusion,\n> + arbitrary suffixes such as _1 are discouraged, as they provide no\n> + meaningful insight into the function's role.\n> +\n\nStray diff from the first round?\n\nThanks,\nTaylor\n"},{"id":"505985","messageId":"CAOLa=ZR7gBCuawUxhQ_j=E5-GqKD+xP0+rPwa6DWAxn9g03UBw@mail.gmail.com","threadId":"62382","inReplyTo":"Zxld62svV9iDCcJJ@nand.local","subject":"Re: [PATCH v2] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-10-23T21:03:55Z","receivedAt":"2024-10-23T21:04:22Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Wed, Oct 23, 2024 at 10:34 PM Taylor Blau wrote:\n>\n> On Wed, Oct 23, 2024 at 09:57:06AM +0200, Karthik Nayak wrote:\n> > + - Function names should be self-explanatory, clearly reflecting their\n> > + purpose or behavior.\n> > +\n> > + The '_1' suffix for function names has historically indicated:\n> > +\n> > + - functions processing one of several elements that all need to be\n> > + handled similarly.\n> > +\n> > + - recursive functions that need to be separated from a setup stage.\n> > +\n> > + To maintain clarity and avoid confusion, such arbitrary suffixes are\n> > + discouraged, as they provide no meaningful insight into the function's\n> > + role.\n> > +\n>\n> I'm still not sold on the suggestion to discourage the use of '_1' in\n> the future, so we may want to further qualify this statement with cases\n> where it is OK (in the spirit of Patrick's \"as long as this is loosely\n> applied\" comment from earlier).\n>\n\nI would say that in some sense goes against my motive for this patch,\nand I still firmly stand by not having '_1' as a suffix. As such, I understand\nthat this might be my biased opinion, so I'll drop the patch overall since\nlike you mentioned, it probably is better to not have anything at all added\nto the documentation.\n\n> > +To maintain clarity and avoid confusion,\n> > + arbitrary suffixes such as _1 are discouraged, as they provide no\n> > + meaningful insight into the function's role.\n> > +\n>\n> Stray diff from the first round?\n>\n\nIndeed :)\n\n> Thanks,\n> Taylor\n\nKarthik\n"},{"id":"505988","messageId":"vshoz6xw3d3kghi7lsag6ldike6pdphkjbst5afxbuvjeyprxd@e4ectkxbh4v7","threadId":"62382","inReplyTo":"Zxld62svV9iDCcJJ@nand.local","subject":"Re: [PATCH v2] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2024-10-23T23:07:08Z","receivedAt":"2024-10-23T23:08:41Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 24/10/23 04:34PM, Taylor Blau wrote:\n> On Wed, Oct 23, 2024 at 09:57:06AM +0200, Karthik Nayak wrote:\n> > + - Function names should be self-explanatory, clearly reflecting their\n> > + purpose or behavior.\n> > +\n> > + The '_1' suffix for function names has historically indicated:\n> > +\n> > + - functions processing one of several elements that all need to be\n> > + handled similarly.\n> > +\n> > + - recursive functions that need to be separated from a setup stage.\n> > +\n> > + To maintain clarity and avoid confusion, such arbitrary suffixes are\n> > + discouraged, as they provide no meaningful insight into the function's\n> > + role.\n> > +\n> \n> I'm still not sold on the suggestion to discourage the use of '_1' in\n> the future, so we may want to further qualify this statement with cases\n> where it is OK (in the spirit of Patrick's \"as long as this is loosely\n> applied\" comment from earlier).\n\nWhen I fist encountered the use of the '_1' suffix, I was also not\nimmediately sure of the intent it was trying to communicate. If I\nremember correctly, there are also existing uses that don't really fit\ninto the two examples mentioned above. One such use I found is\n`handle_revision_arg_1()`[1] where it seems we are just more generally\nfactoring out a subset of the parent functions logic, but keeping the\nfunction name mostly the same. If this is also a usecase, maybe we\nshould instead say something more generic along the lines of:\n\n\"A function suffixed with '_1' indicates that a subset of logic from its\ncorresponding parent function has been factored out and encapsulated in\na separate function.\"\n\nRegarding whether its use should be discouraged, if the convention is\nwell understood and documented it probably isn't much of an issue, but\nfor newcomers it may be another source of confusion as its meaning is\nnot implicitly well understood. I do think its a good first step to\nimprove the documentation here though. :)\n\n-Justin\n\n[1] https://gitlab.com/gitlab-org/git/-/blob/fd3785337beb285ed7fd67ce6fc3d3bed2097b40/revision.c#L2176\n\n\n\n"},{"id":"505996","messageId":"xmqqr086pbna.fsf@gitster.g","threadId":"62382","inReplyTo":"ZxfVwQxMlcJbGt5D@nand.local","subject":"Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-10-24T00:50:17Z","receivedAt":"2024-10-24T00:50:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> I don't disagree that writing \"single\" or \"recursively\" can be\n> considered clearer. I think that the convention to suffix such functions\n> with \"_1()\" is more terse, but saves characters and can avoid awkward\n> line wrapping.\n\nI am reasonably sure that I was the first user of the _1()\nconvention, or at least I was one of them. The reason for the\nchoice of suffix was only because there wasn't anything suitable\nwhen refactoring an existing function foo() into a set-up part and\nits recursive body, so I just kept the set-up part and the single\ncall into the new function in the original foo(), and had to give a\nname to the new function that holds the body of the original logic\nthat was moved from foo().\n\nNeither foo_helper() or foo_recursive() were descriptive enough to\nwarrant such longer suffixes than a simple _1(). They easily can\nget \"help by doing what?\" and \"recursively doing what?\" reaction,\nwhich is a sure sign that the suffixes are not descriptive enough.\n\nThat was the only reason why I picked that \"short-and-sweet but\ncryptic\" suffix.\n\nSurely all of _1(), _helper(), _recursive() are meaningless. If we\nwere to replace existing uses of them, the replacement has to be 10x\nbetter.\n\nHaving said all that, as an aspirational goal, I think it is good to\nencourage people to find a name that is descriptive when writing a\nnew function. I'd refrain from judging if it is way too obvious to\nbe worth documenting (as I am officially on vacation and shouldn't\nbe thinking too much about the project).\n\nThanks.\n"},{"id":"506050","messageId":"Zxp6vKxxwKTb+g5q@nand.local","threadId":"62382","inReplyTo":"xmqqr086pbna.fsf@gitster.g","subject":"Re: [PATCH] CodingGuidelines: discourage arbitrary suffixes in function names","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-24T16:50:04Z","receivedAt":"2024-10-24T16:50:07Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Oct 23, 2024 at 05:50:17PM -0700, Junio C Hamano wrote:\n> Surely all of _1(), _helper(), _recursive() are meaningless. If we\n> were to replace existing uses of them, the replacement has to be 10x\n> better.\n\nWell put. Each of the three are more or less equally meaningless, but\n_1() is an accepted (?) project convention and has the fewest\ncharacters, so I think is a good choice personally.\n\n> Having said all that, as an aspirational goal, I think it is good to\n> encourage people to find a name that is descriptive when writing a\n> new function. I'd refrain from judging if it is way too obvious to\n> be worth documenting (as I am officially on vacation and shouldn't\n> be thinking too much about the project).\n\nYeah, go back to vacation ;-).\n\nThanks,\nTaylor\n"}]}