-
Notifications
You must be signed in to change notification settings - Fork 0
Switch to functional combinators #3
base: master
Are you sure you want to change the base?
Conversation
What's the reasoning behind dropping the external lib? |
@i-am-tom so that users have a |
Edit: I've updated the PR with an invokable implementation of Now, that being said... Cypress' version offers right curry, which I haven't tried to implement. The approach taken by yuyat/curry might be superior in this regard (the curry'd callable is a class with $concat = curry_exactly(3, function ($a, $b, $join = '') {
return $a . $join . $b;
});
assert('a:b' === $concat('a', 'b', ':')); I'd love to be able to do something like this: $concat = curry_right(function ($a, $b, $join) { ... }, ':');
assert('a:b', $concat('a', 'b')); |
Having an immutable `Curry` class allows creating a curry from any callable. Having this as a base class will allow additional functionality such as `Partial` to be added with minimal effort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the long wait! I've left a few comments; nothing too earth-shattering. My main thought is still with curry
- I'm very nervous about adding another curry implementation. It just seems more in-the-spirit to contribute to another curry implementation with anything that we feel is lacking from this one, y'know?
}); | ||
|
||
// The following statements are all equivalent: | ||
assert(6 === $add(1, 2, 3)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was mentioned elsewhere, but $add
should be $sum
``` | ||
|
||
Note that the functions are called **from left to right**. If the opposite is desired, this can be achieved easily: | ||
|
||
```php | ||
<?php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for keeping these was that GitHub syntax highlighting didn't activate without them :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/** | ||
* compose :: (b -> c), (a -> b) -> a -> c | ||
*/ | ||
function compose(callable $f, callable $g): callable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these functions curried? Ignore me - just saw the tests
/** | ||
* k :: a -> b -> a | ||
*/ | ||
function k($x): callable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, if this is being called K
, I'd rather it stayed capitalised - it's a nod to the K
combinator, which only makes sense if capitalised!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should just name it 𝐊
and really confuse people. 😉 Just trying to keep PSR-2 consistency here, function names are lower_snake_case
.
@@ -0,0 +1,77 @@ | |||
<?php | |||
declare(strict_types=1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately this does nothing. It only affects the type checking of functions you call in this file not their declarations.
Strict typing applies to function calls made from within the file with strict typing enabled, not to the functions declared within that file. If a file without strict typing enabled makes a call to a function that was defined in a file with strict typing, the caller's preference (weak typing) will be respected, and the value will be coerced.
You could however declare this on your test files where you actually make the calls and it would then have the desired affect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does more than that, including forcing return type checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But as per the note, it only makes a difference if set in the file calling the methods.
E.g.
// foo.php
<?php
declare(strict_types=1);
class A
{
public function test(int $a): int
{
return $a;
}
public function test2(int $a): int
{
return $this->test($a);
}
}
// bar.php
<?php
require_once __DIR__ . '/foo.php';
$a = new A();
echo $a->test2('1'); // (int) 1
if you take the declare(strict_types=1);
out of the foo.php
file it still returns (int) 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ignore me, that was a terrible test that missed the whole point because '1'
was being type juggled on the way in. :)
@@ -0,0 +1,79 @@ | |||
<?php | |||
declare(strict_types=1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
☝️
*/ | ||
function compose(callable $f, callable $g): callable | ||
{ | ||
return static function ($x) use ($f, $g) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure why this is declared static
it does not impact the function.
For example:
function test($a, $b) {
return function($c) use ($a, $b) {
return sprintf("a: %s; b: %s; c: %s", $a, $b, $c);
};
}
echo test(1, 2)(3); // a: 1; b: 2; c: 3
echo test(4, 5)(3); // a: 4; b: 5; c: 3
and
function test($a, $b) {
return static function($c) use ($a, $b) {
return sprintf("a: %d; b: %d; c: %d", $a, $b, $c);
};
}
echo test(1, 2)(3); // a: 1; b: 2; c: 3
echo test(4, 5)(3); // a: 4; b: 5; c: 3
do the same thing?
In the above example you could make any of the variables $a
, $b
or $c
static however you would then be introducing state into your functions which goes against all the principles of FP.
Or am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this situation, static
does nothing, you are correct.
return $f($g($x)); | ||
}; | ||
} | ||
define('compose', '\PhpFp\compose'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these define
's needed? They work, however the code looks a lot more confusing and the use function
statements are a lot neater.
E.g.
namespace Foo\Bar{
function a() {
return 'A';
}
define('a', '\Foo\Bar\a');
}
namespace Foo\Bass{
echo a; // \Foo\Bar\a
echo a(); // Uncaught Error: Call to undefined function Foo\Bass\a()
echo (a)(); // A
use function Foo\Bar\a;
echo a(); // A
}
Or is there a good use case for these I have missed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed them in #4.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If PR #4 is a latter version of this PR can we close this one to avoid confusion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Tohmua probably not a bad idea. I split them to make it clear what the steps involved were.
I'll leave this open for someone else to finish. We're not going to pursue using php-fp at this point. |
Replace
Combinators
static class with functions.Refs #1