Hash::contains should return true for empty needle

[nrother]

Hash::contains(['a'], []); //false
Hash::contains([], []); //false

This is contradictory to the mathematical definition to a subset. I currently don't see a reason why contains should work this way. (This bug might exist due to a mixup of && and || here)

[markstory]

Changing this would mean that all datasets contain all empty needle values. I'm not convinced that this is a good idea.

[ADmad]

Making this change is also likely to affect existing apps.

[markstory]

Yes. It changes the behavior in a backwards incompatible way.

[thinkingmedia]

If this is important, and I don't think it is. You could add a third parameter called strict that enables this check, but you know it's just extra code that most people won't use.

[nrother]

I tried to use this code to detect if one user has all rights another user has, and this failed, when the first user did not had any rights. I found this very confusing. I might be interesting to find out how often this method is really called with [] for the second parameter and someone is really expecting this to return false. But this might be just me, as I'm from a kind-of-mathematical background.

I'd vote (at least) for a strict parameter.

[nrother]

FYI: Currently all tests pass with my proposed modification. Don't know what is about real-world-applications, though.

[markstory]

@nrother Sounds like we're missing a test case 😄

[lorenzo]

Arrays are not sets in the mathematical definition, so this is completely counter intuitive to how the rest of the language works

[nrother]

@lorenzo Do you have an example of this "rest of the language"? A quick search only reveals array_intersects, which works the way I'd expected it to: array_intersect(['a'], []); //[]

Still can't come up with a use-case where the current behaviour is useful...

[lorenzo]

Yes this, for example:

in_array([], ['a', []]); // true

in_array([], ['a']); // false

[nrother]

in_array([], ['a', []]); // true is a good point, but the import thing there is, that in_array threats the first parameter as a single value to search for, so [] is just any kind of value. I expected Hash::contains as "check if every value from $needle can be found in $haystack". I now found that the documentation says "Determines if one Hash or array contains the exact keys and values of another", which might be a slight different thing, as it does not say "contains all keys and value of another".

I found some other inconsistencies (IMHO) in the framework:

Hash::numeric(['1']) //true
Hash::numeric([]) //false
collection(['1'])->every(function($item) {return is_numeric($item);}) //true
collection([])->every(function($item) {return is_numeric($item);}) //true

But this might be due to the fact that a collection is more a (mathematical) set than a hash. But somehow there seems to be a bit of confusion how to handle empty needles.

[lorenzo]

I agree the second collection example is a bug.

[nrother]

Ok, that was not what I wanted to reach 😄. In the meantime I did some research:

//Java
Arrays.stream(new int[] {1}).allMatch(x -> false); //false
Arrays.stream(new int[] {}).allMatch(x -> false); //true

//C#
new int[]{1}.All(x => false); //false
new int[]{}.All(x => false); //true

#Python
all([False for x in [1]]) #false
all([False for x in []]) #true

//CakePHP (with 64669a9)
collection([1])->every(function() {return false;}) //false
collection([])->every(function() {return false;}) //false

That not what I'd expect as a developer! So for me #8670 clearly made the situation worse. and I could imagine that that even breaks someones code. (It would break mine).

[dereuromark]

Do we want to address this for 4.0?

[markstory]

I think that would be our next opportunity to do it.

[github-actions]

This issue is stale because it has been open for 120 days with no activity. Remove the stale label or comment or this will be closed in 15 days

[othercorey]

I don't think there is anything to do here though.