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

Support callables #10

Merged
merged 4 commits into from
Nov 1, 2021
Merged

Support callables #10

merged 4 commits into from
Nov 1, 2021

Conversation

thgs
Copy link
Contributor

@thgs thgs commented Oct 30, 2021

Fixes #2

Is this what you had in mind?

Also, I am not sure if more should be added to the tests. If yes, any of the below two is preferable?

Separate methods like so

    function it_maps_with_callable()
    {
        $instance = new class
        {
            public function f($value)
            {
                return $value . "bar";
            }
        };

        $this->beConstructedWith("foo");
        $result = $this->map([$instance, 'f']);

        $result->shouldHaveType(Ok::class);
        $result->unwrap()->shouldBe("foobar");
    }

Or performing the check in the same method like so

    function it_maps()
    {
        $this->beConstructedWith("foo");
        $result = $this->map(function ($value) {
            return $value . "bar";
        });

        $result->shouldHaveType(Ok::class);
        $result->unwrap()->shouldBe("foobar");

        $instance = new class
        {
            public function f($value)
            {
                return $value . "bar";
            }
        };

        $result = $result->map([$instance, 'f']);

        $result->shouldHaveType(Ok::class);
        $result->unwrap()->shouldBe("foobarbar");
    }

I am not sure if it is something to test in the first place. It feels like it is.

@prewk

@coveralls
Copy link

coveralls commented Oct 31, 2021

Coverage Status

Coverage remained the same at 100.0% when pulling 47719f1 on thgs:support-callable into 1c1bd9c on prewk:master.

@prewk
Copy link
Owner

prewk commented Oct 31, 2021

Performing the check in the same method is good enough for me!

Thanks, I don't really do PHP anymore so I had to read up on the difference between type hinting Closures and callables haha. Add the test and I'll merge, thanks.

Are you able to do it for Option as well to match?

@thgs
Copy link
Contributor Author

thgs commented Oct 31, 2021

Yes wanted to finalise one of the two first then can do Option too.

Also I was thinking to add more from Haskell Maybe and Either in here.

Rust has got you? I was reading the book and ended up in this package the other day.

@thgs
Copy link
Contributor Author

thgs commented Oct 31, 2021

@prewk tests added here, will do Option next.

Althought the tests do feel a bit redundant now to be honest.

Also, package versions should be ok right? Also since one depends on the other (both ways), could they not be one single package?

@prewk
Copy link
Owner

prewk commented Nov 1, 2021

Thanks a lot!

Yes I think so, I'll bump minorly.

Also I was thinking to add more from Haskell Maybe and Either in here.

Hehe cool, I think there's a myriad of packages nowadays that do this pretty well. But may be wrong. But it's always fun to do your own take on it.

Rust has got you? I was reading the book and ended up in this package the other day.

I really like it, but for work I'm deep in Typescript at the moment and will be for quite some time!

@prewk prewk merged commit 5a5d803 into prewk:master Nov 1, 2021
@thgs thgs deleted the support-callable branch December 14, 2021 01:20
@thgs
Copy link
Contributor Author

thgs commented Dec 14, 2021

@prewk Thanks for merging here too.

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

Successfully merging this pull request may close these issues.

Support callables
3 participants