Return to Revisions

Following directly on from the excellent suggestions by @MartinR, I would firstly rename CodeGen to PasswordGenerator – which is a much clearer name for it. It also fits better with the API design guidelines, which discourages the use of abbreviations ("Gen" instead of "Generator"):

• Avoid abbreviations. Abbreviations, especially non-standard ones, are effectively terms-of-art, because understanding depends on correctly translating them into their non-abbreviated forms.

The intended meaning for any abbreviation you use should be easily found by a web search.

Then, I would consider making the different password generating options actual instance properties of PasswordGenerator (therefore we need to make it a struct, as enumerations cannot contain stored properties).

This increases the flexibility of the API, as it allows for easy re-use of a particular password generating instance. It could even, for example, serve as a model for a view controller (e.g you could have a passwordGenerator property with a didSet observer that updates the UI with a new randomly generated password upon the options being changed, with various UI elements being able to change those options).

So we now have:

struct PasswordGenerator {
 
 private static let base = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
 private static let lowerChars = "abcdefghijklmnopqrstuvwxyz"
 private static let numberChars = "1234567890"
 private static let specialChars = "!@#$%^&*"
 
 var length: Int
 var useLowercase: Bool
 var useNumbers: Bool
 var useSpecialChars: Bool
 
 init(length: Int, useLowercase: Bool = true,
 useNumbers: Bool = false,
 useSpecialChars: Bool = false) {
 
 self.length = length
 self.useLowercase = useLowercase
 self.useNumbers = useNumbers
 self.useSpecialChars = useSpecialChars
 }
 
 func generate() -> String {
 
 var letters = PasswordGenerator.base
 
 if useLowercase {
 letters += PasswordGenerator.lowerChars
 }
 if useNumbers {
 letters += PasswordGenerator.numberChars
 }
 if useSpecialChars {
 letters += PasswordGenerator.specialChars
 }
 
 let characters = Array(letters.characters)
 let numChars = UInt32(characters.count)
 
 let result = String((0..<length).map { _ in
 characters[Int(arc4random_uniform(numChars))]
 })
 
 return result
 }
}

I've also renamed the generatePassword() method to generate(), as it's now clear from the type name what the method is generating.

However, those Bool properties to control the characters used aren't particularly elegant – I would recommend encapsulating them in a CharacterSet nested type, which we can then conform to OptionSet:

extension PasswordGenerator {
 
 struct CharacterSet : OptionSet {
 
 static let lowercase = CharacterSet(rawValue: 1 << 0)
 static let uppercase = CharacterSet(rawValue: 1 << 1)
 static let numbers = CharacterSet(rawValue: 1 << 2)
 static let symbols = CharacterSet(rawValue: 1 << 3)
 
 static let letters: CharacterSet = [.lowercase, .uppercase]
 static let alphanumeric: CharacterSet = [.letters, .numbers]
 
 let rawValue: Int
 var characters: Set<Character> {
 
 var characters = Set<Character>()
 
 if contains(.lowercase) {
 characters.formUnion("abcdefghijklmnopqrstuvwxyz".characters)
 }
 
 if contains(.uppercase) {
 characters.formUnion("ABCDEFGHIJKLMNOPQRSTUVWXYZ".characters)
 }
 
 if contains(.numbers) {
 characters.formUnion("1234567890".characters)
 }
 
 if contains(.symbols) {
 characters.formUnion("!@#$%^&*".characters)
 }
 return characters
 }
 }
}

(Note that this will conflict with Foundation's CharacterSet, but given as it's a nested type, this shouldn't be problematic)

This also lets us define some custom character sets, such as letters and alphanumeric, which are a combination of other sets – as well as encapsulate the logic to produce the set of characters to work with, as this could well be re-used. Note that we're now using a Set to represent the collection of characters we can choose from, as it better describes the collection (unique characters that aren't ordered).

Now we just have to refactor PasswordGenerator to use our nested CharacterSet type instead of the Bool properties:

struct PasswordGenerator {
 
 var length: Int
 
 var characterSet: CharacterSet {
 didSet {
 // re-compute the characters array after the character set changing.
 characters = Array(characterSet.characters)
 }
 }
 
 // cached characters.
 private var characters: [Character]
 
 // default options of just using letters (upper and lowercase)
 init(length: Int, characterSet: CharacterSet = .letters) {
 self.length = length
 self.characterSet = characterSet
 self.characters = Array(characterSet.characters)
 }
 
 func generate() -> String {
 // we cannot generate a password if there's no characters
 // to generate it from.
 guard !characters.isEmpty else {
 return ""
 }
 
 let characterCount = UInt32(characters.count)
 
 let result = (0 ..< length).map { _ in
 characters[Int(arc4random_uniform(characterCount))]
 }
 
 return String(result)
 }
}

You can see we're now:

• Caching the characters to generate the password from (re-creating upon the character set changing, with a didSet observer)
• Using our custom .letters option as the default character set to generate from.
• Guarding against an empty character option set being selected by just returning an empty string.

We can now simply use it like so:

var passwordGenerator = PasswordGenerator(length: 16, characterSet: .alphanumeric)
print(passwordGenerator.generate()) // GcWrH81pYU46m4lx
passwordGenerator.length = 20
print(passwordGenerator.generate()) // 9SNiPlgYqRCpvpP96cuT
passwordGenerator.characterSet.subtract(.lowercase)
print(passwordGenerator.generate()) // YX1WELZEC7GN4L2JTAVR

I have included the full code in a gist for your convenience.