jkoudys / immutable.php

Immutable collections, with filter, map, join, sort, slice, and other methods. Well-suited for functional programming and memory-intensive applications. Runs especially fast in PHP7.
MIT License
346 stars 20 forks source link

Map #11

Closed jroenf closed 7 years ago

jroenf commented 7 years ago

I submitted issue #9 with regards to the map method and it recently got closed. Thank you for the changes!

I think we can do better and make the method comply to the description of the native php function array_map. It is a variadic function, having a variable-length argument list. This will make the map method more versatile and feel more intuitive for php developers. The new approach will also provide for the specific use case currently implemented. The array indexes and $this (current object) can be injected like this:

$index = range(0, count($immArray) - 1); $immArray->map(function($value, $i) use ($imArray) { ... ;}, $index);

Although my changes did not make any existing test fail, I guess it will break backward compatibility. Version bump 🎉 😉

jroenf commented 7 years ago

I'd like to ask you @jkoudys:

Wow, Travis works great. It feels good having 'All checks passed'.

jkoudys commented 7 years ago

Hi @jroenf , happy new year!

I have been considering making this the last version that supports <PHP7. A big motivation for this project is to play nicely with dependency injection, and define interfaces around collections. To that end, return-typing would be really nice. There are also a few places where I use Reflection (or call_user_func), which is damn slow and ugly to read, but could be neatly replaced by spread-arguments.

I'd like to deliberately avoid making things too much like the native PHP functions. It's a principle of this project that many of the array functions have inconsistent interfaces and do things in a nonsensical way. It's also true that many were written before certain features/language syntax was available, so now we have better tools to do the same thing. Multiple collections passed to a map made sense back when Closures hadn't arrived, or weren't nearly as powerful. Now that we have them, it reads more naturally to put them in the use, which is even more native-PHP (as part of the syntax spec for function, vs the definition of array_map), so it follows it's a more natural approach.

e.g.

$colours = ImmArray::fromArray(['red', 'yellow', 'blue']);
$flowers = ImmArray::fromArray(['rose', 'daffodils', 'orchids']);
$labels = $colours->map(function ($colour, $i) use ($flowers) {
    return $colour . ' ' . $flowers[$i];
});

The only downside I can see is that callbacks written for array_map can't be directly dropped-in to an ImmArray#map, so you'd need to wrap in a closure.

// e.g. this callback is defined elsewhere
function show_Spanish($n, $m)
{
    return("The number $n is called $m in Spanish");
}

// Wrong
$numbers->map('show_Spanish');

// Need wrapper
$numbers->map(function ($number, $i) use ($spanish) {
    return show_Spanish($number, $spanish[$i]);
});

Not as nice, but it's a case that comes up rarely enough on any code new enough to be using this lib anyway.

jroenf commented 7 years ago

Thank you and a very happy new year to you too 🍾

I'd like to deliberately avoid ... the array functions have inconsistent interfaces and do things in a nonsensical way.

Thank you for explaining the purpose of this library. Not copying bad design makes sense 👍