[prev in list] [next in list] [prev in thread] [next in thread] 

List:       perl5-porters
Subject:    Re: [perl #75350] PATCH: revamp ibcmp_utf8() for efficiency, clarity
From:       demerphq <demerphq () gmail ! com>
Date:       2010-05-31 8:19:09
Message-ID: AANLkTim4xL2KPR1tJgkUQT_vAwGori2gXDFNXCe6uI_p () mail ! gmail ! com
[Download RAW message or body]

On 30 May 2010 22:39, karl williamson <public@khwilliamson.com> wrote:
> demerphq wrote:
>>
>> On 29 May 2010 01:28, karl williamson <public@khwilliamson.com> wrote:
>>>
>>> demerphq wrote:
>>>>
>>>> On 25 May 2010 20:24, karl williamson <perlbug-followup@perl.org> wrote:
>>>>>
>>>>> # New Ticket Created by  karl williamson
>>>>> # Please include the string:  [perl #75350]
>>>>> # in the subject line of all future correspondence about this issue.
>>>>> # <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=75350 >
>>>>>
>>>>>
>>>>> The first times I looked at this routine, it seemed incomprehensible.
>>>>>  I
>>>>> had to come back to it to fix a bug after working on other parts of
>>>>> Perl
>>>>> for a while, and it wasn't as foreign as before.  But I had to really
>>>>> understand it, so I started adding comments, and discovered that it was
>>>>> checking a number of things each time in the loop that could have  been
>>>>> figured out once.  So I ended up revamping it, keeping the same
>>>>> algorithm.
>>>>
>>>> Somebody please figure out how to clone Karl so we have a backup.
>>>> Thanks.
>>>>
>>>> On a more serious note...
>>>>
>>>> This looks much nicer to read than the original. ++ to you.
>>>>
>>>> +Returns true if the strings s1 and s2 differ case-insensitively, false
>>>> +if they are equal case-insensitively.  Note that this is the complement
>>>> of what
>>>> +you might expect (perhaps it would have been better to name it
>>>> C<ibncmp_utf8>).
>>>>
>>>> Seems to me that cmp routines always behave like this, they return -1
>>>> for inorder but different, 0 for different, and 1 for reverse order.
>>>
>>> I agree that I was wrong, and ibncmp is not a good name, but this routine
>>> returns only TRUE or FALSE, unlike the cmp routines.  A better comparison
>>> would be with functions like memNE.  I'm always startled when I read code
>>> calling this function, because it looks like it's doing the wrong thing
>>> to
>>> me, and I'm a firm believer that names can help or hinder.
>>
>> Me too. If you think this function should be renamed, and we can still
>> proved a back-compat alias should something actually be using it, then
>> go ahead and rename it.
>>
>> Be bold!
>
> OK; I'll try.  But I'm now thinking your tries for case insensitive matching
> will cause this function to actually not be needed in the core?

Possibly, but that is a longer term consideration that I think should
be omitted when considering the merits of patches that we have here
and now.

Yves



-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

[prev in list] [next in list] [prev in thread] [next in thread]