ericpoe / haystack

Forget Haystack vs Needle order, the object IS the Haystack. Haystack is a library that allows for pipelining and immutable structures.
MIT License
38 stars 1 forks source link

FunctionalInterface::filter() should support passing keys/both in all PHP versions #14

Open johnkary opened 8 years ago

johnkary commented 8 years ago

Supplying the $flag parameter to FunctionalInterface::filter(callable $func = null, $flag = null); currently relies on PHP 5.6 constants \USE_KEY and \USE_BOTH to decide whether the callback is called with the value, key or both <value, key>. This functionality should instead be available for all PHP versions supported via composer.json require statement, currently >= 5.5.9.

I see two possible approaches here...

1) Haystack adds its own constants and uses them for filter($func, null $flag = null).

2) Haystack adds two new methods that don't require calling code to provide the flags.

ericpoe commented 8 years ago

The constants USE_KEY and USE_BOTH belong to Haystack. The PHP flags are ARRAY_FILTER_USE_KEY and ARRAY_FILTER_USE_KEY

The tests pass for PHP 5.5, so I'm not sure that I understand what the problem is.

johnkary commented 8 years ago

You're right. My 1am-brain wasn't thinking correctly. However in the docblock for FunctionalInterface::filter() it mentions PHP 5.6 being required to pass the USE_BOTH flag. https://github.com/ericpoe/haystack/blob/master/src/Functional/FunctionalInterface.php#L36

The bigger issue I was trying to identify was FunctionalInterface::filter() accepting a flag as part of its interface feels imprecise. It prescribes functionality that can't be bound to the method signature--as in could two FunctionalInterface implementations be guaranteed to support the same flags? A separate implementation could decide they want to create a whole new flag USE_BOTH_BACKWARDS where their callback argument order is flipped to ($key, $value). Their implementation honors the interface but the implementation is not compatible with all FunctionalInterface implementations.

What if instead of accepting a flag argument the filter($fn) method always accepted a callback function($value, $key)? This is the behavior of USE_BOTH. If the calling code only cares about the value (in my opinion this would be most common) it is valid to pass a callback whose signature is function($value).

Many functions in Laravel's Collection class pass both ($value, $key) to the callback. While functions in Doctrine's ArrayCollection class don't make this concession only passing ($value) to the callback.

ericpoe commented 8 years ago

I like the idea. I spent a lot of time trying to figure out why ARRAY_FILTER_USE_BOTH expected callback($value, $key) rather than a more natural callback($key, $value); I finally found the reason by asking one of the internals devs who worked on that feature. I don't want others to look at Haystack and ask why the order of parameters is so funky. Also I would love to be able to not use flags in this method. This method should just work without the end-user needing to anything special.

There are 4 different types of array_filter and Haystack is trying to cover all of them:

I can picture easy ways to implement the no-callback and fully-loaded-callback possibilities. However, how should we discern between when only a $key is used and when only a $value is used in the callback? If I check for array_key_exists($key, $array) to determine if callback($key) is meant rather than callback($value) and yet callback($value) was meant and it just so happens that a supplied $value has the same name as a $key?

Any ideas?