Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ArrayObject::isAssoc() #58

Closed
wants to merge 3 commits into from
Closed

Add ArrayObject::isAssoc() #58

wants to merge 3 commits into from

Conversation

leocavalcante
Copy link
Member

Checks whether the given array wrapped by ArrayObject is associative (uses string keys like a hashmap).

@deminy
Copy link
Member

deminy commented Oct 8, 2020

The Swoole team had discussed the implementations of this particular method two months ago. It's always inefficient when implemented in PHP, like the one implemented in Laravel ( https://github.com/laravel/framework/blob/v8.9.0/src/Illuminate/Collections/Arr.php#L389 ). Although there were thoughts like comparing keys in reverse order, it doesn't help much in worse cases.

Let's see how others think. Thanks @leocavalcante for the PRs!

@twose
Copy link
Member

twose commented Oct 13, 2020

The best way is php/php-src#6070

$index = count($array);
reverse_foreach($array as $key => void) {
    if ($key !== --$index) {
        return false;
    }
    return true;
}

@deminy
Copy link
Member

deminy commented Jan 31, 2021

According to this PHP RFC (Add array_is_list(array $array): bool), a new PHP function array_is_list() will be added to PHP 8.1. This new function can be found in the Docker image for PHP nightly (8.1-dev only):

docker run --rm -ti phpdaily/php:8.1-dev php -r 'echo function_exists("array_is_list") ? "yes" : "no", "\n";'

As discussed, the same function written in PHP is much less efficient compared to a built-in PHP function like array_is_list(). Thus, we prefer not to add method ArrayObject::isAssoc() to the Swoole library; developers can add and use this method (or some variations) manually in their projects when needed.

Thanks again to @leocavalcante for submitting the PR and for contributing to the open source community!

@deminy deminy closed this Jan 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants