Add function where()#122
Conversation
There was a problem hiding this comment.
Document weither it’s an "any properties" or "all properties" proposition?
| * @param array $propertyMap | ||
| * @return bool | ||
| */ | ||
| function matches($object, array $propertyMap) |
There was a problem hiding this comment.
Can you inline the function so that we are not exposing it in the Functional\ namepace
There was a problem hiding this comment.
Directly inlining it makes it too much of a mess.
The only thing I can think of is making it 2 closures inside the main function like so:
<?php
namespace Functional;
use Functional\Exceptions\InvalidArgumentException;
function where($objects, array $propertyMap)
{
$readProperty = function($object, $property) {
//...
};
$matches = function($object, array $propertyMap) use ($readProperty) {
//...
};
//... implementation of where()
}| throw new \InvalidArgumentException(sprintf('Expected property or method %s in %s', $property, get_class($object))); | ||
| } | ||
| } elseif (is_array($object)) { | ||
| return $object[$property]; |
There was a problem hiding this comment.
What if the property does not exist? Wouldn’t this issue a strict warning?
There was a problem hiding this comment.
Yes it would. I could throw a OutOfBoundsException and catch it in matches() and return false for no match.
| * @param $property | ||
| * @return mixed | ||
| */ | ||
| function read_property($object, $property) |
There was a problem hiding this comment.
Can you inline this function as well?
| */ | ||
| function read_property($object, $property) | ||
| { | ||
| if (is_object($object)) { |
There was a problem hiding this comment.
Other scenarios that come to mind are magic methods (__isset and __get). Symfony’s property path component also supports isFoo() and getFoo() but that might be overkill.
There was a problem hiding this comment.
I don't think there is a good way to determine if a magic property exist. The object should have implemented an __isset() method but there is no guarantee that it does and it might have different behavior for null which currently works fine.
I suggest to just not support magic properties, maybe document it explicitly.
| if (property_exists($object, $property)) { | ||
| return $object->$property; | ||
| } elseif (method_exists($object, $property)) { | ||
| return call_user_func([$object, $property]); |
|
|
||
|
|
||
| ## zip() | ||
| ## zip() & zip_all() |
There was a problem hiding this comment.
Can you split the zip_all() doc change into another PR?
2 participants
Issue #120
The file contains multiple functions, I don't think there is any other file which does this. Should I split it up?
Many extensions are possible. Support for ArrayAccess would be one.
It now throws an exception when a property is not found but I can imagine cases where it is better to return no match instead. Like in the case of dynamic properties or when checking for a nested property and the containing object is null.