3
\$\begingroup\$

I wrote a function which picks a two different random elements from a closed range of ints and returns them as a tuple.

I would like to take a your feedback, if the solution is a "Clever", "Best practice" or "non of them"

func pickTwoRandomElementsFromRange(_ range: ClosedRange<Int>) -> (Int, Int)? {
    guard range.count >= 2 else { return nil }

    let firstElement = range.randomElement()
    var secondElement = range.randomElement()

    while firstElement == secondElement  {
        secondElement = range.randomElement()
    }
    if let firstElement = firstElement, let secondElement = secondElement {
        return (firstElement, secondElement)
    }

    return nil
}
\$\endgroup\$
3
\$\begingroup\$

Naming

Swift naming conventions are listed in the API Design Guidelines. With respect to “Omit needless words” and “Strive for Fluent Usage” I would suggest to call the function

func randomPair(from range: ClosedRange<Int>) -> (Int, Int)?

Simplify the code

Your method is fine, but can be simplified a bit. First, the return values from randomElement() can safely be forced-unwrapped because it is guaranteed at that point that the range is not empty. That makes the final optional binding obsolete:

func randomPair(from range: ClosedRange<Int>) -> (Int, Int)? {
    guard range.count >= 2 else { return nil }

    let firstElement = range.randomElement()!
    var secondElement = range.randomElement()!

    while firstElement == secondElement {
        secondElement = range.randomElement()!
    }
    return (firstElement, secondElement)
}

Alternatively one can use a repeat-loop, so that the second element is assigned only at one place, that is a matter of personal preference:

func randomPair(from range: ClosedRange<Int>) -> (Int, Int)? {
    guard range.count >= 2 else { return nil }

    let firstElement = range.randomElement()!
    var secondElement: Int
    repeat {
        secondElement = range.randomElement()!
    } while firstElement == secondElement

    return (firstElement, secondElement)
}

But actually the same can be achieved without a loop, if we pick the second element from a reduced range and adjust it if necessary:

func randomPair(from range: ClosedRange<Int>) -> (Int, Int)? {
    guard range.count >= 2 else { return nil }

    let firstElement = range.randomElement()!
    var secondElement = range.dropLast().randomElement()!
    if secondElement >= firstElement {
        secondElement += 1
    }

    return  (firstElement, secondElement)
}

Generalizations

Now we have a function to get a random pair from a closed range. But what about open ranges? What about ranges of other types? In fact it is not difficult to implement the same algorithm for arbitrary (random access) collections. The only difference is that we pick random indices first:

func randomPair<C>(from collection: C) -> (C.Element, C.Element)? where C: RandomAccessCollection {
    guard collection.count >= 2 else { return nil }

    let firstIndex = collection.indices.randomElement()!
    var secondIndex = collection.indices.dropLast().randomElement()!

    if secondIndex >= firstIndex {
        collection.formIndex(after: &secondIndex)
    }

    return (collection[firstIndex], collection[secondIndex])
}

This covers closed ranges, open ranges, arrays etc:

randomPair(from: 0...10)
randomPair(from: 0..<10)
randomPair(from: ["a", "b", "c", "d"])

Alternatively one can implement this as an extension method for RandomAccessCollection so that it closely resembles the existing randomElement() method:

extension RandomAccessCollection {
    func randomPair() -> (Element, Element)? {
        guard count >= 2 else { return nil }

        let firstIndex = indices.randomElement()!
        var secondIndex = indices.dropLast().randomElement()!
        if secondIndex >= firstIndex {
            formIndex(after: &secondIndex)
        }
        return (self[firstIndex], self[secondIndex])
    }
}

(0...10).randomPair()
(0..<10).randomPair()
["a", "b", "c", "d"].randomPair()
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

I think Martin’s answer (+1) checks most of the boxes.

First, that having been said, I wonder whether you’ve considered using this within non-countable ranges. For example, would you ever want to use it to generate a pair of floating point values (e.g. two angles whose values are between zero and 2π)? Is that part of what you might want to do? If so, a different pattern would be called for.

Second, I might advise against tuple return type. As soon as you start using a function designed to return two values within a range, someone is going to want to then return three values from the range. A tuple is just too constraining. I’d be inclined to use a set or an array, instead. And even if you wanted to never return more than two values, I’d still ask whether a struct might make more sense, where the functional intent of these two values is more clear (e.g. ordered values, unordered? etc.).

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.