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

Differentiate between closures #42

Merged
merged 1 commit into from
Oct 31, 2019
Merged

Differentiate between closures #42

merged 1 commit into from
Oct 31, 2019

Conversation

sergejostir
Copy link
Contributor

@sergejostir sergejostir commented Oct 26, 2019

This package uses function name in a hash calculation. For closures, function name will always be "namespace{closure}", therefore calculated hash will be always the same, if arguments also match. This raises a problem, since once() will be run only once regardless of the method it is nested in and return the same result across methods.

What I did here is, that the hashing method checks, if once() was called from within a closure and instead of using generic function name, it uses the line number of once() call.

Here is an example, how I got an idea for even using once() in a closure:

public function getSomething($element)
{
    $getData = function () {
        return once(function () {
            // An expensive function call that does not depend on
            // method's arguments.
        });
    };

    retrun $getData()[$element];
}

If I was to use once() outside the closure, it would take $element argument into consideration, so every time the method would be called with a different argument, once() would call its closure. Now with once() being in a closure, the arguments (there are none in the example) are always the same and the calculated hash will match every time).

I could of course extract this expensive function call in its own method, but sometimes you would only need that part one time, so declaring a separate method seems overkill and using closure just seems more "right".

I also included two tests which demonstrate the functionality of this PR.

Bonus: If you wrap once() in a closure, you can also have multiple such occurances per method giving you different results. Currently you can only have one once() per method because every next one will return you the result of the first one.

@freekmurze freekmurze merged commit 6aa2c6b into spatie:master Oct 31, 2019
@freekmurze
Copy link
Member

Great PR, thanks!

@sergejostir sergejostir deleted the pr/support-closures branch October 31, 2019 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants