Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] TODOs for version 2.0 #141

Open
lstrojny opened this issue May 9, 2017 · 19 comments
Open

[RFC] TODOs for version 2.0 #141

lstrojny opened this issue May 9, 2017 · 19 comments
Labels

Comments

@lstrojny
Copy link
Owner

lstrojny commented May 9, 2017

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 ([IHNFIWIWD] The compose function is executed left to rightΒ #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 lstrojny changed the title TODOs for version 2.0 [RFC] TODOs for version 2.0 May 9, 2017
@lstrojny lstrojny added the RFC label May 9, 2017
@MarcoWorms
Copy link

Suggestion: curry all functions by default (#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 :)

@jdreesen
Copy link

❗ 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

@lstrojny
Copy link
Owner Author

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

@Erikvv
Copy link
Contributor

Erikvv commented May 29, 2017

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.

@tPl0ch
Copy link

tPl0ch commented Dec 14, 2017

@lstrojny What about lazy evaluation? Is that planned?

@lstrojny
Copy link
Owner Author

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

@redbeardcreator
Copy link

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
Copy link

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.

@lstrojny
Copy link
Owner Author

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

@redbeardcreator
Copy link

@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?

@lstrojny
Copy link
Owner Author

lstrojny commented Feb 1, 2018

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

@ragboyjr
Copy link

ragboyjr commented Feb 5, 2018

@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};

@lstrojny
Copy link
Owner Author

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

@ragboyjr
Copy link

ragboyjr commented Aug 11, 2018

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

https://github.com/krakphp/fn

@lstrojny lstrojny mentioned this issue Feb 8, 2019
@lstrojny lstrojny pinned this issue Feb 8, 2019
@igneus
Copy link

igneus commented Aug 10, 2019

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

@lstrojny
Copy link
Owner Author

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

@igneus
Copy link

igneus commented Aug 10, 2019

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

@ragboyjr
Copy link

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.

@tzkmx
Copy link

tzkmx commented Nov 26, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants