Dictionary extension 'random()' is untested for empty dictionaries and fails for this untested case

[dfrib]

The generally non-deterministic random() method of the Dictionary extensions is implemented as follows:

/// EZSE: Returns the value of a random Key-Value pair from the Dictionary
public func random() -> Value {
    let index: Int = Int(arc4random_uniform(UInt32(self.count)))
    return Array(self.values)[index]
}

As far as I can see, this method is never tested for the deterministic cases where the dictionary is empty or contains a single element. For the former—if applied onto an empty dictionary—it will yield a runtime exception, as the Array(self.values) array will be empty.

Leaving aside this corner case crash, as an additional comment, the method could be implemented making use of the already existing Optional-returning random() method of the Array extension. The following re-factoring of the Dictionary extension would be equal to the existing one w.r.t. functionality:

public func random() -> Value {
    return Array(self.values).random() ?? values.first!
}

Where also the corner case crash case becomes apparent. Possibly worth letting also this method (I'm not sure how you go about to implement breaking changes), be an Optional-returning one, much like its sibling from the Array extension. E.g.:

public func random() -> Value? {
    return Array(self.values).random()
}

Also, worth adding a test for the random() method of Dictionary, much like is present for its sibling in Array. E.g. (given the re-factoring to Optional return above):

func testRandom() {
    // deterministic _value_ return ('nil' and '1')
    XCTAssertNil([:].random())
    let singleEntryDictionary = ["one": 1]
    (1...10).forEach { _ in
        XCTAssertEqual(singleEntryDictionary.random(), Optional<Int>.some(1))
    }
    
    // non-deterministic value return, but deterministic optional case ('.some(_)')
    (1...10).forEach { _ in
        XCTAssertNotNil(firstdic.random())
    }
}

[Khalian]

Addressing it here : #420

[Khalian]

Good callout. I do not see any other way other to make it optional. I will test the other randoms as well to see if they have issues like this.