haystack icon indicating copy to clipboard operation
haystack copied to clipboard

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

Open johnkary opened this issue 8 years ago • 3 comments

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).

  • null - Pass value.
  • FunctionalInterface::USE_KEY - Pass key.
  • FunctionalInterface::USE_BOTH - Pass both.

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

  • filter($func, null $flag = null) -- Passes container value to callback unless $flag given. $flag can be one of null || FunctionalInterface::USE_KEY || FunctionalInterface::USE_BOTH. Return true preserves key => value in new container.
  • filterByKey($func) -- Passes container key to callback. Implementation delegates to filter() and supplies the correct constant. Return true preserves key => value in new container.
  • filterByBoth($func) -- Passes container key as first callback argument, value as second argument. Implementation delegates to filter() and supplies the correct constant. Return true preserves key => value in new container.

johnkary avatar May 25 '16 06:05 johnkary

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.

ericpoe avatar May 25 '16 15:05 ericpoe

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.

johnkary avatar May 26 '16 14:05 johnkary

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:

  • no callback provided
  • callback($value)
  • callback($key)
  • callback($key, $value)

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?

ericpoe avatar Jun 05 '16 19:06 ericpoe