Re: Re: cvs: php-src /ext/standard array.c

On Feb 18, 2009, at 6:52 PM, Moriyoshi Koizumi wrote:

On Thu, Feb 19, 2009 at 4:51 AM, Andrei Zmievski <andrei@gravitonic.comwrote:

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.

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.

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?

- Ian