HiveBrain v1.2.0
Get Started
← Back to all entries
patternswiftMinor

Big Nerd Ranch Bronze Challenge: Disallow Alphabetic Characters

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
ranchchallengenerdalphabeticbigdisallowbronzecharacters

Problem

I just completed BNR's bronze challenge for disallowing alphabetic characters, and would love some feedback on what I did right and wrong, and how I can improve my code for reusability and maintainability.

Here's a snippet of instruction from their book, iOS Programming: The Big Nerd Ranch Guide (5th Edition) (Big Nerd Ranch Guides):


Currently, the user can enter alphabetic characters either by using a Bluetooth keyboard or by pasting copied text into the text field. Fix this issue. Hint: you will want to use the NSCharacterSet class.

I basically have a text field that takes numeric input. I allow the user to enter digits 0-9, inclusive with negative numbers and one decimal point. What the app does is convert Fahrenheit to Celsius, but that's not entirely relevant.

My ViewController is conforming to the UITextFieldDelegate protocol, and I'm implementing:

optional public func textField(textField: UITextField, shouldChangeCharactersInRange range: NSRange, replacementString string: String) -> Bool


Here's my snippet:

func textField(textField: UITextField, shouldChangeCharactersInRange range: NSRange, replacementString string: String) -> Bool {
    let existingTextHasDecimalSeparator = textField.text?.rangeOfString(".")
    let replacementTextHasDecimalSeparator = string.rangeOfString(".")

    if string.characters.count == 0 {
        return true
    }
    else {
        if let _ = string.rangeOfCharacterFromSet(NSCharacterSet(charactersInString: "-0123456789.")) {
            if existingTextHasDecimalSeparator != nil && replacementTextHasDecimalSeparator != nil {
                return false
            }
            else {
                return true
            }
        }
        return false
    }
}

Solution

Two problems in your code

You check if the replacement string contains at least one allowed
character, so that pasting "1A" into the text field would be accepted.
Instead you should check if the string contains no invalid character:

if string.rangeOfCharacterFromSet(
    NSCharacterSet(charactersInString: "-0123456789.").invertedSet) != nil {
    return false
}


(I prefer a comparison != nil over an optional binding to the
unused variable _ in this situation.) Note that this makes the initial
check for a non-empty string obsolete.

In order to prevent two decimal separators (such as "1.2.3"), you check if both
the existing string and the replacement string contain a period. But
this prevents the replacement of – for example – "1.2" by "3.4" with
copy/paste. To handle that correctly, you have to build the
new string and check that:

if let existingString = textField.text as NSString? {
    let newString = existingString.stringByReplacingCharactersInRange(range, withString: string)
    // check if `newString` contains more than one dot `.`
    // ...
}


Possible improvements

The decimal separator is not always the
dot character ".". In Germany for example (and many other countries),
floating point numbers are written as "12,34" with a comma as separator, and that would be denied with your code.

You could determine the correct localized decimal separator at
runtime:

let separator = NSNumberFormatter().decimalSeparator


and use that instead of the hard-coded period. But that still does not
cover all invalid input, such as "12-34-56".

There is a better (and simpler) solution: Check if the new string
after replacement can successfully be converted to a number, using
a NSNumberFormatter. That will recognize valid numbers according to
the user's locale correctly:

func textField(textField: UITextField, shouldChangeCharactersInRange range: NSRange, replacementString string: String) -> Bool {

    // Get current string as `NSString`
    guard let existingString = textField.text as NSString? else { return false }
    // Perform replacement:
    let newString = existingString.stringByReplacingCharactersInRange(range, withString: string)
    // Check for empty result ...
    if newString.isEmpty {
        return true
    }
    // ... or valid number:
    let fmt = NSNumberFormatter()
    fmt.numberStyle = .DecimalStyle
    return fmt.numberFromString(newString) != nil
}


Since creating a number formatter is "expensive" (compare
NSFormatter - NSHipster), you
can improve that further by creating the formatter only once – for example as a static property.

Code Snippets

if string.rangeOfCharacterFromSet(
    NSCharacterSet(charactersInString: "-0123456789.").invertedSet) != nil {
    return false
}
if let existingString = textField.text as NSString? {
    let newString = existingString.stringByReplacingCharactersInRange(range, withString: string)
    // check if `newString` contains more than one dot `.`
    // ...
}
let separator = NSNumberFormatter().decimalSeparator
func textField(textField: UITextField, shouldChangeCharactersInRange range: NSRange, replacementString string: String) -> Bool {

    // Get current string as `NSString`
    guard let existingString = textField.text as NSString? else { return false }
    // Perform replacement:
    let newString = existingString.stringByReplacingCharactersInRange(range, withString: string)
    // Check for empty result ...
    if newString.isEmpty {
        return true
    }
    // ... or valid number:
    let fmt = NSNumberFormatter()
    fmt.numberStyle = .DecimalStyle
    return fmt.numberFromString(newString) != nil
}

Context

StackExchange Code Review Q#138096, answer score: 3

Revisions (0)

No revisions yet.