Add addCallback() method #474
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jesse-r-s-hines
force-pushed
the
add-callback
branch
from
6951dd3
to
3af8371
Compare
setCallback() replaces previous callbacks making you have to repeat any logic in the "base case' definition callback if you have a callback on one of the groups. This commit adds an addCallback() method which can be used to add a callback while still letting earlier callbacks be called as well. addCallback() will add the callback to a stack, and then all of the callbacks will be called when creating the object. If any of the callbacks return false the model won't be persisted a second time. Calling setCallback() after addCallback() will clear the callback stack before it sets the new callback. This will maintain backwards compatibility with the previous behavior of setCallback().
jesse-r-s-hines
force-pushed
the
add-callback
branch
from
3af8371
to
7b2c2a7
Compare
GrahamCampbell
requested changes
Jan 12, 2021
| * | ||
| * @var callable[] | ||
| */ | ||
| private $callbackStack = []; |
There was a problem hiding this comment.
Why do we need this as well as a callback variable. Can we remove the callback property?
There was a problem hiding this comment.
We don't need both, I used the callback property to avoid recreating the lambda on every getCallback(). That may be premature optimization though.
There was a problem hiding this comment.
I've removed the callback field and just used callbackStack directly in getCallback()
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
setCallback() replaces previous callbacks making you have to repeat any
logic in the "base case' definition callback if you have a callback on
one of the groups. This commit adds an addCallback() method which can
be used to add a callback while still letting earlier callbacks be called
as well.
addCallback() will add the callback to a stack, and then all of the callbacks
will be called when creating the object. If any of the callbacks return false
the model won't be persisted a second time.
Calling setCallback() after addCallback() will clear the callback stack
before it sets the new callback. This will maintain backwards compatibility
with the previous behavior of setCallback().