On 08.12.2025 09:43, Heikki Linnakangas wrote:
> On 08/12/2025 10:37, Chao Li wrote:
>> Hi Hackers,
>>
>> In a lot places, there are logic of appending comma separators in a
>> pattern like:
>>
>> ```
>> for (int i = 0; i < len; i ++)
>> {
>> if (i > 0)
>> appendStringInfoString(", ");
>> appendStringInfo(some-item);
>> }
>>
>> ```
>> This pattern uses an "if" check and two appendStringInfoString() to
>> build a comma-delimited string.
>>
>> This can be simplified as:
>>
>> ```
>> const char *sep = "";
>> for (int i = 0; i < len; i ++)
>> {
>> appendStringInfo("%s%s", sep, some-item);
>> sep = ", ";
>> }
>> ```
>> The new pattern avoids the "if" check, and combines two
>> appendStringInfoString() into a single appendStringInfo(). I think the
>> new pattern is neater and faster.
>>
>> The old patterns are used in a lot of places, and there are some
>> usages of the new pattern as well. Instead of creating a big cleanup
>> patch, I just applied the new pattern to a single file for now to see
>> if the hacker group likes this change.
>
> It's a matter of taste, but I personally prefer the "old" pattern with
> an explicit if() statement more. And I don't think it's worth the code
> churn to change existing code either way.
>
> - Heikki
It's also very likely not faster because now appendStringInfo() is
called one more time and appendStringInfo() also needs to check if the
string is empty or not, which requires an if in some form or shape (e.g.
a loop that bails immediately).
--
David Geier
