On Thu, Feb 19, 2009 at 3:14 PM, Ian Eure <i...@digg.com> wrote:
> On Feb 18, 2009, at 6:52 PM, Moriyoshi Koizumi wrote:
>
>> On Thu, Feb 19, 2009 at 4:51 AM, Andrei Zmievski <and...@gravitonic.com>
>> wrote:
>>>
>>> Moriyoshi Koizumi wrote:
>>>>
>>>> As I said earlier, the function is never supposed to be used with
>>>> objects. Therefore, we cannot declare it to be broken, and any change
>>>> to the behavior anyway leads to a huge BC break. I got a report that
>>>> claims the reporter's real-world application behaves strangely with
>>>> the latest release candidate.
>>>
>>> Should we have array_unique_for_non_strings() then or something?
>>
>> That'd be stupid. What is reasonable is to make it default to
>> SORT_STRING... Or is there any strong reason to make the default
>> SORT_REGULAR now that you can specify SORT_REGULAR to array_unique()
>> throught the second argument?
>>
> Because it's bad to require an argument to make a function do the right
> thing; it should do the right thing by default.

As I said, whether it is right or not is pretty much a context-depndent matter.

>>>> That said, I'm not really against making SORT_REGULAR default for
>>>> later versions than 5.2.x as long as *appropriate notices* are
>>>> provided, while I strongly disagree for 5.2.x.
>>>
>>> What sort of notices do you propose? At runtime or in the docs?
>>
>> You even didn't mention that the behavior would be changed starting
>> with 5.2.9 in the document, instead you simply added the description
>> for the second optional argument that defaults to SORT_REGULAR, as if
>> it was the default long before. That's absolutely the thing we should
>> not do.
>>
> Why would it be necessary? SORT_REGULAR is a superset of SORT_STRING, yes?
> If the function should "only be used with strings" (prior to this patch), as
> you claim, that's not a BC break. Unless you're arguing that we should
> preserve BC for code calling functions in unsupported ways.

If SORT_REGULAR was a superset of SORT_STRING, there would have been
no problem in the first place. See the original bug report to see what
is to be broken.

>> Eitherway, if we were to make such change, I think we should at least
>> make the second argument mandatory.
>>
> Wouldn't this be an even bigger BC break than fixing the existing (broken)
> behavior?

A little sarcasm, the point here is that it'd be better off making the
*incompatible* application not work at all than still allowing them to
silently malfunction, but such a decision is just a bit less absurd
than changing the default, as you pointed out.

Anyway that's basically not a fix IMO.

Moriyoshi

>
>  - Ian
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to