Become a MacRumors Supporter for $50/year with no ads, ability to filter front page stories, and private forums.

abcdefg12345

macrumors 6502
Original poster
Jul 10, 2013
281
86
I wrote some code that formats a textfield to include numbers only, thousands separators and limit amount of digits to 30, was wondering if someone can make some improvements on it reduce the code, fix bugs if there are any and if there is a more efficient way to use such code with multiple fields.

Code:
import Foundation
import Cocoa

class sampleclass: NSObject, NSTextFieldDelegate {
  
    @IBOutlet weak var MyTextField: NSTextField!
    var StringBeingChanged = ""
  
    override func controlTextDidChange(_ notification:Notification) {
        if notification.object as? NSTextField == MyTextField {
            //format the field
            self.FormatSelectedField()
        }
    }
  
    func FormatSelectedField() {
        //make textfield numbers and decimal only
        let aSet = CharacterSet(charactersIn:"0123456789.").inverted
        MyTextField.stringValue = MyTextField.stringValue.components(separatedBy: aSet).joined(separator: "")
      
        //limit field to 30 digits
        if MyTextField.stringValue.characters.count > 30 {
            MyTextField.stringValue = String(MyTextField.stringValue.characters.dropLast((MyTextField.stringValue.characters.count - 30)))
        }
      
        // numberformatter
        let numberFormatter: NumberFormatter = NumberFormatter()
        numberFormatter.groupingSeparator = ","
        numberFormatter.groupingSize = 3
        numberFormatter.usesGroupingSeparator = true
        numberFormatter.maximumFractionDigits = 30
        numberFormatter.decimalSeparator = "."
        let character = "."
      
        //value before format for later use
        StringBeingChanged = MyTextField.stringValue
      
        //format textfield
        MyTextField.stringValue  = numberFormatter.string(for: NSDecimalNumber(string: MyTextField.stringValue))!
      
        // if original value contains decimal and new value does not, add a decimal to new value
        if !(MyTextField.stringValue.characters.contains(".")) && (StringBeingChanged.characters.contains(".")) {
            MyTextField.stringValue = String(format: "%@%@", MyTextField.stringValue, character)
        }
      
        let string1 = 0.00
        let string2 = 0.00
        let subtract = 0.00

        //count number of zeroes after format
        string1 = Double(MyTextField.stringValue.components(separatedBy: "0").count)
      
        //count number of zeroes before format
        string2 = Double(StringBeingChanged.components(separatedBy: "0").count)
      
        //get the number of zeroes missing
        subtract = string2 - string1
      
        //add missing zeroes
        if string2 > string1 {
            MyTextField.stringValue = "\(MyTextField.stringValue)\("".padding(toLength: Int(subtract), withPad: "0", startingAt: 0))"
        }
    }
}
 
You didn't specify which version of Swift you're using. This will affect which methods the String class has. Newer Swift versions have more methods directly available on a String.


Here's a few comments on general structure. This is not an exhaustive list.

1. There doesn't need to be a MyTextField property. The target of the transformation is already being passed via the notification:Notification object, so simply take the target from that, and pass it to FormatSelectedField. This will allow one instance to be reused with multiple textfields.

2. The property StringBeingChanged can be local to the function FormatSelectedField. It only needs to exist while the function is active (executing). It's perfectly safe to have it be re-instantiated each time the function runs. Again, this will allow one instance to be reused safely with multiple textfields.

3a. The local variable numberFormatter is needlessly created each time FormatSelectedField is called. Move it into a property (narrow its scope to something less than public), and instantiate it when the sampleclass is created. You could pass it to the object's initializer, to allow creating with a given NSNumberFormatter.

3b. Same applies to aSet.

4. All lower case is a poor choice for class names. The name "sample class" says nothing about what an object of the class does. Name the class something that makes sense for what it does. Even "MyCustomTextfieldFormatter" is better than "sampleclass". Some other variables are also poorly named: too generic (aSet, character), misleading (string1 is not actually a string, but a length), etc.

5. The var's string1, string2, and subtract only need to be integer types. They represent counts, and there's no such thing as a fractional count of characters. That is, doubles are misleading and unnecessary. 'subtract' is superfluous; simply use the expression (string2-string1) where it occurs in the one place you have it.

6. The String type has a 'length' and a 'contains' method. It's not necessary to refer to 'characters' (may depend on Swift version).

7. The constant 'character' initialized to "." is unused. You probably intended to use it in the places where the literal "." currently appears. A better name is also advised (e.g. decimalPointChar). Also, decimal-point and thousands grouping are locale-dependent. Your class should honor those.

8. It may be more efficient (faster) to not use the textfield's stringValue as much, instead using a local working string for all the transformations, only setting the textfield at the end. This may be insignificant to overall speed, but it could improve readability.


All the above will be obviated if you attach an NSNumberFormatter directly to an NSTextField or its cell.
Google search terms: swift nstextfield number formatter
or: swift nscontrol formatter


Finally, none of the above are necessary if the current class meets all your current operational needs. In short, there's no point in improving it unless the current code poses a measurably significant problem. Effort spent in improving unmeasurable or insignificant costs has no payoff. The way to think of it is, "If this class takes zero time and zero memory, how would that improve the program as a whole?"

Conversely, if your goal is to learn more about how to design effective classes in the first place, then all the points above should be present in your initial class. They're pretty elementary, since they emerge out of understanding the basic scope and nature of what's happening. "Scope" includes where variables exist, they're lifetime, visibility, and how/why they change or don't change. "Nature" covers things like recognizing that counts are always integers, not doubles, or that strings can be operated on directly for things like length and presence of a character, and using the simplest type that solves the problem.
 
Register on MacRumors! This sidebar will go away, and you'll see fewer ads.