Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Zend\Authentication\Result custom result codes not possible #4452

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants

What's the point of this in the Result constructor:

    if ($code < self::FAILURE_UNCATEGORIZED) {
        $code = self::FAILURE;
    } elseif ($code > self::SUCCESS ) {
        $code = 1;
    }

It pretty much nullifies the usefulness of any classes that extend result using custom codes without making a custom constructor, and seems to be there for no discernible reason.

@ghost ghost assigned weierophinney May 9, 2013

Owner

weierophinney commented May 9, 2013

Actually... this change would only make sense if we were using static:: instead of self:: for resolution -- and even then, the fact is that AuthenticationService is checking for the specific Result constants, which means self:: is correct.

Can you clarify what you feel needs to change, please, @JustinBuser ?

Contributor

mwillbanks commented May 10, 2013

This portion of the code actually drives me insane also; I extend it by using the constructor right now.

I have several different codes that I use today and throw differently in the constructor. This kills the open/close principle in a bad way :(

Remove it, it serves no purpose other than to override values that are being explicitly set, ergo: shouldn't need to be overwritten. Can you cite some reason it needs to be there?

weierophinney added some commits May 9, 2013

[#4452] Use constant values, not integers
- Use constant values, to make extension possible
Remove code munging
- Per the comments, changing the code value if it fell outside the range of the
  defined constants does not change how isValid() works, and does not allow for
  custom constants in extending classes, unless you also override the
  constructor.
Owner

weierophinney commented May 23, 2013

@mwillbanks Okay, updated -- tests still pass, so should be fine. Please review! :)

mwillbanks added a commit that referenced this pull request May 23, 2013

Perfection, 👍

weierophinney added a commit to zendframework/zend-authentication that referenced this pull request May 14, 2015

[zendframework/zendframework#4452] Use constant values, not integers
- Use constant values, to make extension possible

weierophinney pushed a commit to zendframework/zend-authentication that referenced this pull request May 14, 2015

weierophinney pushed a commit to zendframework/zend-authentication that referenced this pull request May 14, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment