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

Improvements to Dispatcher class #567

Merged
merged 16 commits into from
Apr 4, 2024
Merged

Improvements to Dispatcher class #567

merged 16 commits into from
Apr 4, 2024

Conversation

fadrian06
Copy link
Contributor

Tired of declaring after filters with the first parameter without doing anything?

DELETE IT :D, nothing will be broken.

Dispatcher now supports after filters with the only useful parameter.

Flight::after('myMethod', function (&$params, &$output) {
  // $params Even if it is used it does not affect, but it had to be declared yes or yes even if it does nothing.
});

Now...

Flight::after('myMethod', function (&$output) {
  // goodbye useless $params 
});

IMPORTANT: both ways work exactly the same. Only one is shorter and cleaner.

To even make it even cleaner, you could change the filter to a PHP 7.4 arrow function.

Flight::after('myMethod', fn (&$output) => doMagic($output));

Just make sure that what you do in the arrow function doesn't return false, unless you want to make other after filters for the same mapped method not run.

Copy link
Collaborator

@n0nag0n n0nag0n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you did some improvements which if they work out will help the code be cleaner which I appreciate! Your code editor is messing up a few things that need to be corrected though. And the only other thing is that I don't want to introduce dependencies into Flight, that's part of the selling point.

.gitignore Show resolved Hide resolved
flight/core/Dispatcher.php Show resolved Hide resolved
flight/core/Dispatcher.php Outdated Show resolved Hide resolved
flight/core/Dispatcher.php Outdated Show resolved Hide resolved
flight/core/Dispatcher.php Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
flight/core/Dispatcher.php Show resolved Hide resolved
flight/util/ReturnTypeWillChange.php Show resolved Hide resolved
@fadrian06 fadrian06 closed this Apr 2, 2024
@n0nag0n
Copy link
Collaborator

n0nag0n commented Apr 2, 2024

@fadrian06 Why'd you close the PR? It's a good PR just needs a few tweaks

@fadrian06 fadrian06 reopened this Apr 3, 2024
flight/core/Dispatcher.php Outdated Show resolved Hide resolved
@n0nag0n n0nag0n merged commit d1584a8 into master Apr 4, 2024
@n0nag0n n0nag0n deleted the dispatcher-refactor branch April 4, 2024 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants