functional-php icon indicating copy to clipboard operation
functional-php copied to clipboard

[RFC] TODOs for version 2.0

Open lstrojny opened this issue 8 years ago • 19 comments

Mission

  • Fix all the things that I got wrong in 1.x
  • Avoid accidental breakage as much as possible

❗ Dangerous 👍 Easy

Topics

  • ❗ Fix order of compose arguments (#117)

    • In 1.x
      • Introduce compose_2 that fixes the argument order
      • Deprecate composeand ask people to migrate to compose_2
      • Introduce pipe that does what compose does in 1.x
    • In 2.0
      • Rename compose to compose
      • Deprecate compose_2 and ask users to use compose instead
  • Fully fledged PHP 7.1 support

    • 👍 Use type hints everywhere
      • Should we return iterable or array for container return values?
    • ❗ Should we use generators for lazy evaluation?
  • ❗ Do we want to continue passing value, index, collection?

    • E.g. map suffers from the same problems JavaScript’s map suffers from
> ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10"].map(parseInt)
[ 0, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, 10 ]
  • ❗ Switch argument order from collection, function to function, collection
    • Is the more common signature in the functional world
    • Reads better as "map F over L"

lstrojny avatar May 09 '17 23:05 lstrojny

Suggestion: curry all functions by default (https://github.com/lstrojny/functional-php/issues/136) and leave the data for the last argument (much like ramda, they really did get that right)

Also, for the value, index, collection problem i wouldn't say it's an issue. It's just an edge case that can be easily worked around, I wouldn't break this clean interface because of some edge cases :)

MarcoWorms avatar May 10 '17 03:05 MarcoWorms

❗ Do we want to continue passing value, index, collection? E.g. map suffers from the same problems JavaScript’s map suffers from

> ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10"].map(parseInt)
[ 0, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, 10 ]

A unary function that wraps a function of any arity in a function that accepts exactly 1 parameter would help to prevent those edge cases.

So you could do:

> ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10"].map(unary(parseInt))
[ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ]

See: https://lodash.com/docs/4.17.4#unary and http://ramdajs.com/docs/#unary

jdreesen avatar May 10 '17 05:05 jdreesen

@MarcoWorms we had a couple of requests for auto-currying (#136, #41, #118) but my stance is that I can’t think of a way to do this efficiently. This is why I am not considering this for 2.0 right now.

@jdreesen indeed it can be worked around but it’s just not a proper map as found in most functional languages. Even PHPs own array_map() does it correctly. So while handing in value, key and collection is convenient, it’s wrong. The unary function sounds like a nice addition anyway.

lstrojny avatar May 10 '17 10:05 lstrojny

We've talked about this before but one of the pitfalls I keep stepping into is that when you filter or select over an array with contiguous key it is no longer contiguous.

Erikvv avatar May 29 '17 07:05 Erikvv

@lstrojny What about lazy evaluation? Is that planned?

tPl0ch avatar Dec 14 '17 05:12 tPl0ch

@tPl0ch I have no vision around that for now. Do you have an idea on how to do that?

lstrojny avatar Dec 20 '17 10:12 lstrojny

I personally like the idea of just passing the value into the function for e.g. map(). However, I have had occasion to use the index and collection in the past. Specifically, I've used the index for non-numerically keyed arrays and the collection in cases where I, for some reason, use other values in the collection. I can't think of any specific examples right now, though. Thinking about it in the context of this discussion, though, makes me think it was probably a bad idea.

Removing the collection parameter can be worked around as:

map($list, function ($item) use ($list) { /* do something with $list */ });

It's not the most elegant solution, but it works and call attention to the fact that maybe this isn't really the right way to do it.

The index can be handled with something like:

map(
    zip(array_keys($list), array_values($list)),
    function ($spot) { list($index, $value) = $item; /* ... */ }
);

Then you could use idea of map_2() to promote the new version, providing these workarounds. Most use cases wouldn't have to change anything.

redbeardcreator avatar Jan 29 '18 03:01 redbeardcreator

I don't know if I like the idea of changing parameter order. To me personally, the order seems correct. It's closest to $collection->map(function () { /* ... */ }). I'd live with either. However, since the parameter order is currently (collection, function), I'd be leery of changing it.

I want to say iterable instead of array, but I'm still mostly stuck on PHP 5.6 and haven't had a chance to test how that would work. Hopefully we'll be there by the end of the year (I'm really starting to miss out on package updates.) I know it would probably help in some circumstances.

I would say generators are a Good Thing™. The biggest problem I have with them is being able to reuse the source. As an example, there's no way to rewind the result set from the MySQL flavor of PDO (unlike the mysql_* functions). If you process said result set via a generator, you've then lost the original. Usually that's ok, but sometimes it's not. At the moment I'm using iter\callRewindable() from nikic\iter to work around the problem. That package is also a good example of how to use generators for functional code.

Finally, +1 to typehinting as much as possible. Despite the fact I'm stuck on PHP 5.6.

BTW, kudos on the great package...I've been using it for quite some time.

redbeardcreator avatar Jan 29 '18 04:01 redbeardcreator

@redbeardcreator on readability: I would love to read it as "map foo over items" but I am not yet sure what the economics of such a change would be. My hunch is, that I want to provide some sort of conversion tool to support the migration. Already out of plain self-interest as we use it quite a bit at @InterNations.

lstrojny avatar Jan 29 '18 09:01 lstrojny

@lstrojny, a conversion tool would help a lot. Would you want to create one from scratch or do something like a fixing sniff for PHP_CodeSniffer?

redbeardcreator avatar Feb 01 '18 03:02 redbeardcreator

@redbeardcreator I haven’t thought about it a lot yet but I probably need something like https://github.com/Roave/BetterReflection to safely convert e.g. map invocations.

lstrojny avatar Feb 01 '18 22:02 lstrojny

@lstrojny regarding auto currying, how hard would it be to just maintain curried versions of certain functions in a separate namespace under curried or something similar?

`use function Functional{map, curried\map};

ragboyjr avatar Feb 05 '18 10:02 ragboyjr

@ragboyjr hard to tell but I guess they could be generated so it might work

lstrojny avatar Aug 11 '18 13:08 lstrojny

@lstrojny , check out what I’ve done with krakphp/fn. I auto generate curried versions of functions.

https://github.com/krakphp/fn

ragboyjr avatar Aug 11 '18 17:08 ragboyjr

Have you considered adding function name constants like ihor/nspl has? They are very handy when passing the library functions around.

igneus avatar Aug 10 '19 10:08 igneus

@igneus we have Functional::… class constants but introducing namespaced, top level constants seems reasonable

lstrojny avatar Aug 10 '19 12:08 lstrojny

I must admit I haven't noticed the \Functional\Functional::... constants before, but "namespaced, top level constants" would definitely make me happier.

use Functional as f; // single import statement

f\map(/*...*/); // direct invocation
$myHigherOrderFunction(f\map); // passing reference around

igneus avatar Aug 10 '19 12:08 igneus

This was also done in krak/fn library: https://github.com/krakphp/fn

The constants there are auto generated which make it very easy to keep in sync.

ragboyjr avatar Aug 10 '19 16:08 ragboyjr

I've just sent my proposal for a Pipe in order to get this functionality ;-) long before 2.0 :-) Pull: #200

  • In 1.x ...
    • Introduce pipe that does what compose does in 1.x

tzkmx avatar Nov 26 '19 00:11 tzkmx