Bronze Challenge

#1

My solution to the Bronze Challenge…


func textField(textField: UITextField, shouldChangeCharactersInRange range: NSRange, replacementString string: String) -> Bool {
        let existingTextHasDecimalSeparator = textField.text?.rangeOfString(".")
        let replacementTextHasDecimalSeparator = string.rangeOfString(".")
        
        let replacementTextHasInvalidChars = string.rangeOfCharacterFromSet({
            let validChars = NSMutableCharacterSet.decimalDigitCharacterSet()
            validChars.addCharactersInString(".")
            return validChars.invertedSet
        }())
        
        if existingTextHasDecimalSeparator != nil &&
           replacementTextHasDecimalSeparator != nil ||
           replacementTextHasInvalidChars != nil {
            return false
        } else {
            return true
        }
    }
#2

The reason that you use closures to initialize a property of a class is to keep the code that initializes the property grouped with the property, and because you can’t have arbitrary expressions at the class level, you wrap the statements in a closure. Your closure does not accomplish anything like that, so I find that it needlessly complicates the code. Instead of this:

let replacementTextHasInvalidChars = string.rangeOfCharacterFromSet({ let validChars = NSMutableCharacterSet.decimalDigitCharacterSet() validChars.addCharactersInString(".") return validChars.invertedSet }())

you could have written this:

let validChars = NSMutableCharacterSet.decimalDigitCharacterSet() validChars.addCharactersInString(".") let replacementTextHasInvalidChars = string.rangeOfCharacterFromSet(validChars)

I haven’t timed it, but I would expect that the latter is more efficient as well: you don’t have to create an (anonymous) function, then call it.

Also, compound conditionals are not something you want to use, rather they are required to express certain conditions. While writing something like this:

if existingTextHasDecimalSeparator != nil && replacementTextHasDecimalSeparator != nil || replacementTextHasInvalidChars != nil { return false } else { return true }

requires a lot of thought and may engender job security, if someone who isn’t as smart as you needs to read the code, or you yourself have to decipher the conditional many months after you wrote the code, it requires too much mental gymnastics to figure out. Keeping conditionals simple is always better:

        if existingTextHasDecimalSeparator != nil
           && replacementTextHasDecimalSeparator != nil  {
            return false
        }

        if replacementTextHasInvalidChars != nil {
            return false
        }

        return true
#3

Well, thank you for your input. (7stud7stud), I specially liked the one about the compound conditional, and I agree, it is much more readable.
About the define and call closure, I think it is a matter of taste…

Take a look at this: http://www.apeth.com/swiftBook/ch02.html#SECdefineandcall

It is part of the iOS 9, Matt Neuburg’s fundamentals; and it seems that he likes to use them.

#4

Well, if it’s between me and Matt Neuburg I would go with Matt Neuburg every time if I were you!

However…his argument for why he likes to use a closure for a define-and-call pattern seems lame to me. He says he doesn’t like having to create the para variable, and then he creates a closure and creates the para variable inside the closure anyway. In addition, all the extra indenting required in his example makes me cringe: I find that iOS names are so long that it’s hard to fit code on a full line much less a double indented line.

In any case, thanks for the link. I will read the other stuff there.