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

Added custom validity checking delegate method to PAPasscode #6

Closed
wants to merge 6 commits into from

Conversation

JThramer
Copy link

So the general idea behind PAPasscodeViewController:checkPasscodeValidityWithEntry: is to enable the developer to implement a custom validity test for the entered passcode using whatever arbitrary means they see fit.

The reason why we implemented this delegate method is because for our solution, we store the passcode as a one-way hash that we obviously cannot unhash. Using this delegate method, we can hash the candidate entry and see if the hashes match.

Hope this helps!

(this is attempt #2, where the first attempt was closed by me because it was half-baked )

@dhennessy
Copy link
Owner

Sorry, I can't accept the pull request in its current form. The feature idea is a good one, but I have a few issues with the pull request:

  1. Code unrelated to the feature shouldn't be included (like changing the navigation bar color)
  2. There are some other code formatting changes (moving braces to a new line) - it's better to remain consistent with the existing code.
  3. The use of the ternary operator in your new code makes it hard to read. A better approach would be to create a new local method to validate the passcode, which then uses the delegate or does a local comparision.

As I said, the idea is a good one so I'll give to thought to adding it. I'm also happy to consider another pull request.

Thanks,
Denis

@JThramer
Copy link
Author

I'll try cleaning it up some and removing our implementation-specific stuff (such as our black navbar) when i have a minute. Thanks for the reply.

@Nr9
Copy link

Nr9 commented Mar 21, 2013

I was going to do a pull request for exactly the same reason. This is a great feature.

@JThramer
Copy link
Author

JThramer commented Mar 2, 2023

Closing as (extremely) stale.

@JThramer JThramer closed this Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants