-
Notifications
You must be signed in to change notification settings - Fork 16
Some refactoring in order to reduce the number of redirects #23
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
base: master
Are you sure you want to change the base?
Conversation
redirects-optimization Conflicts: Controller/Component/WizardComponent.php
| protected function _setCurrentStep($step) { | ||
| if (!in_array($step, $this->steps)) { | ||
| return; | ||
| } |
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'm not a fan of checks like this. I mean I'd be concerned where they got the step from if it's an invalid choice, this is sort of an error you should never reach.
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 do not remember exactly why that was needed. I think that has something to do with internal workings of this component to prevent setting the future step that is not in $this->steps yet. An infinite loop happened without this check. Notice while (current($this->steps) != $step) below. The clients cannot call _setCurrentStep() directly as it is protected so there is no need to worry.
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.
That would be the exact reason to leave comments in the code. Otherwise I would take out the code rather than just hiding it under private.
No description provided.