Re: [PATCH v2] Rename suffixcmp() to has_suffix() and inverse its result

+1 for the change. I find the resulting code easier to understand, too.


On 05.11.2013, at 05:57, Christian Couder [off-list ref] wrote:

As suffixcmp() should not be used as an ordering comparison function,
and anything-cmp() ought to be usable as an ordering comparison function,
suffixcmp() should be renamed to something that doesn't end with "cmp".

has_suffix() is a straightforward name for such a function, except
that with such a name callers will expect that it will return 1
when the suffix is present and 0 otherwise.

So we need to also inverse the value returned by this function ti

match what the callers will expect, because suffixcmp() like all
anything-cmp() returns 0 when the suffix is present and 1 or -1
otherwise.

As we inverse the value returned by the function, we also have
to inverse the ways its callers are using its returned value.

s/inverse/invert/  (multiple times)

Taking one step back, shouldn't the commit message rather explain the new status, instead of referring so much to the past? If I imagine somebody reading this in a year, they might not even know suffixcmp (e.g. if they joined the project after this patch was merged).

How about something like this:

--- 8< ----

Rename suffixcmp() to has_suffix() and invert its result

Now has_suffix() returns 1 when the suffix is present and 0 otherwise.

The old name followed the pattern anything-cmp(), which suggests
a general comparison function suitable for e.g. sorting objects.
But this was not the case for suffixcmp().

--- 8< ----

By the way, a much stronger reason why suffixcmp is not suitable than that it is not clear what it would mean, is that it is not transitive. I.e. for an ordering you would want that if a<b and b<c then a<c. This is /was not the case for suffixcmp:

  suffixcmp("3", "31") = -1  (because "31" is longer than "3"), so "3" < "31"
  suffixcmp("31", "2") = -1  (because "1" < "2"), so "31" < "2"
but
  suffixcmp("3", "2") = 1    so "3" > "2"

Signed-off-by: Christian Couder <redacted>
---
Hi Junio and Peff,

So here is a new version of the patch to rename
suffixcmp() into has_suffix(). We now inverse the
result of the function as we rename it.

This patch should be added to or squashed into the
patch series that removes postfixcmp().

[...]