-
Notifications
You must be signed in to change notification settings - Fork 378
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
scratch work of sanitizers and pre/post validation #92
Closed
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
6a3fc44
scratch work of sanitizers and pre/post validation
eabe9a9
more work on new validation design
8920b06
made revisions to validate flow
dd49c7d
Merge pull request #100 from jpotts18/new-validator-design-revision
davepatterson b03a592
Update README.md
davepatterson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
// | ||
// Preparator.swift | ||
// Validator | ||
// | ||
// Created by David Patterson on 2/22/16. | ||
// Copyright © 2016 jpotts18. All rights reserved. | ||
// | ||
|
||
import Foundation | ||
|
||
public protocol Sanitizer { | ||
func sanitize(textField: UITextField) | ||
} |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// | ||
// Sanitizers.swift | ||
// Validator | ||
// | ||
// Created by David Patterson on 2/22/16. | ||
// Copyright © 2016 jpotts18. All rights reserved. | ||
// | ||
|
||
import Foundation | ||
|
||
public class TrimLeadingAndTrailingSpacesSanitizer: Sanitizer { | ||
public init() {} | ||
public func sanitize(textField: UITextField) { | ||
textField.text = textField.text?.stringByTrimmingCharactersInSet(NSCharacterSet.whitespaceCharacterSet()) | ||
} | ||
} |
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
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
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
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
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.
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.
Hey. I'm new here so don't want to step on any toes but I have a comment about this. I think the Sanitizer should take a string and output a string instead of taking the text field directly. This would avoid mutating state in addition to following similar api design as the Rule protocol. Additionally this would allow you to remove the sanitizers for loop and use reduce to apply the string transformations.
Additionally, I'm not sure the sanitized string should ever be reapplied back onto the text field. One example of why this isn't a good idea is if I'm trying to validate the textfield as the field is being typed in. If the sanitizer is applied (as the code is currently written) it will be changing the text field as I'm typing which could be a frustrating experience for the user.
Perhaps there should be a distinction between sanitizing text to prepare it for the rules validation process and formatting text for display purposes. In my opinion this library shouldn't be in the business of formatting the text. In that sense applying the sanitization output back on to the text field doesn't make sense.
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.
After thinking about this some more, and looking back at the original ticket for this, I've come to the conclusion that the point of the sanitizers is to format the text. This being the case I don't feel that the sanitizers should run before validation. They should run after validation and they should only be applied on the textfields that have passed validation.
Any sanitization that needs to be done on the strings in textfields in order for rules to run would be avoiding the ultimate purpose of the rules in the first place.. to validate that the contents of the string don't violate whatever constraint that rule is trying to uphold. And if that rule doesn't care about something in the string because it doesn't violate the constraint then it should just ignore it. So why sanitize before the rule?
But this takes be back to my previous comment.. I don't think a text validation library, that markets itself as such, should be in the business of formatting text for display purposes.