Add accessor with bounds check to Array

collection

(Alexander Momchilov) #41

It's strictly more limit thought. That basically acts as if you got an optional, but you could only ever nil-coalesce it. You wouldn't be able to write code like

if let item = items[safe: i] {
    doSomething()
} else {
    doSomethingElse()
}

(Jens Persson) #42

With a more descriptive (but much too verbose) index-label, that would be:

if let item = items[useOptionalLayerInsteadOfTrappingToHandlePossibleOutOfBounds: i] {
    doSomething()
} else {
    doSomethingElse()
}

Again, items[safe: index] implies that

  • index is safe (in what way?)
  • default subscripting items[index] is unsafe (not true)

We'd have to use items[safer: index], with the motivation above, to not vilify the default subscript, but that'd still look weird.

I agree with:

And perhaps some of the motivating use cases would be better off using a Dictionary with Int keys instead of an Array, eg:

var items = [Int : String]()
items[1] = "foo"
items[3] = "bar"
items[4] = "baz"

let someIndex = Int.random(in: 0 ... 5)

if let item = items[someIndex] {
    print("Does something with item \"\(item)\".")
} else {
    print("Does something without an item at index \(someIndex).")
}

(Tino) #43

Looking at the common argument against calling this construct "safe" or "checked", I just thought about a good reason for inclusion:
Afaics, the straightforward implementation will always perform superfluous checks.
First, there is the test wether we you have to return nil, and afterwards, the regular subscripting (which doesn't know about the first step) will do the same comparisons again.
I expect that an implementation in the stdlib wouldn't use subscripting, but the same primitive mechanism that decides wether to return an element or to trap.


(Jens Persson) #44

The optimizer would/should probably do away with any immediate redundant bounds checks.


(Chéyo Jiménez) #46

I gree its a problem. I just dont agree on the aproach.

what about

collection.containsIndex(someIntIndex) {

doSomething(collection[someIntIndex])

} else {

doSomethingElse()

}

I dont know if that is in scope for swift 5.


(Karl) #47

I don't agree with adding this at all, but if it is going to be added, I see no reason to make it a subscript. You won't be able to set an element to an out-of-bounds index, so passing self as inout makes no sense.

For example:

var arr = [1, 2, 3]
arr[safe: 999] = 42 // won't work, will have to trap.

Instead, I think the correct spelling of this would be:

func element(at: Int) -> Element?

(Chéyo Jiménez) #48

that should not compile.


(Karl) #49

Why not?


#50

Because you can't write a sound and properly working setter for the subscript.


(Karl) #51

Yeah that’s the point I was making... we must have got our wires crossed


(Benjamin Mayo) #52

Swift allows you to have read-only subscripts. You don’t need a trapping setter.


#53

Oh you can write a non trapping setter for this, but you can't write a setter that is working correctly. :wink:


(Karl) #54

The issue is that people are twisting themselves in to all kinds of yoga positions trying to think of a subscript label for this. The most popular suggestions of "safe"/"unsafe"/"checked"/"unchecked" are not just bad, they are all wrong: using the word "safe" or "unsafe" would conflict with the meaning already established by types like UnsafePointer and using "checked" or "unchecked" would incorrectly imply that the existing subscript doesn't bounds-check its argument (which, coincidentally, would be unsafe if it was true).

The naming challenge becomes much easier once you consider this to be a function - and there is no downside to doing so. Since it cannot have a sensible setter, there is no need for self to be passed as inout (which is the only difference between subscripts and functions/computed properties).


(Tino) #55

But it's not impossible to write a sane setter:

  • A function could throw (I don't know if throwing subscripts will ever be added)
  • Appending might be just fine (but it might also be irritating...)
  • You could limit the setter to a custom DefaultConstructible protocol and fill the missing indexes with zeros, empty strings or whatever is returned from Element.init()

Even without the irritation, I wouldn't like a label like "checked" - imho subscript labels should be avoided, and if that's not possible, I strongly prefer nouns...

See above - the ability to write a throwing setter is one reason why I'd prefer a function over a subscript ;-)


(Nate Cook) #56

I agree with this! I know talking about this brings the wrath of the complexity gods, but I use an element(n:) method on Sequence to make it easy to get the nth element. That's not the same as what we're talking about, but it's in the same ballpark.

This is a little unrelated to the rest of the discussion, but you're in luck — Slice and any other slice type (a collection that is its own subsequence) do have popFirst. Your example should work today. :+1:


#57

Sorry, it is. At least to my knowlege. The problem is that the new value is inferred to be an optional of Element, not the Element type itself.

Consider this dummy implementation.

extension Array {
    
    public subscript(safe index: Index) -> Element? {
        
        get {
            guard
                index >= startIndex && index < endIndex
                else { return nil }
            return self[index]
        }
        set (newValue) {
            guard
                index >= startIndex && index < endIndex,
                let newValue = newValue
                else { return }
            self[index] = newValue
        }
    }
}

Using the setter

var numbersA: [Int] = [0, 1, 2, 3, 4]

numbersA[safe: 2] = 9

var x = numbersA[safe: 2] // 9

works fine as expected. It does nothing if the index is out of bounds.

But you can write a wrong (!) assignement. This compiles, but at the end behaves somehow correctly since it does nothing.

numbersA[safe: 2] = nil

var y = numbersA[safe: 2] // 9 (whatever value that was before)

But what, if the elements of the array are themself optionals?

var numbersB: [Int?] = [0, 1, 2, 3, 4]

numbersB[safe: 2] = nil

var z = numbersB[safe: 2] // 2 Huu!?

The result is clearly incorrect.

I don't know of any way to solve this problem generally for optional and non optional elements in swift. If somebody has a solution, I'd love to hear it. Perhaps there is something else missing in swift, because I think something like this should be doable.


(Jens Persson) #58

If we try to look past all the magic, pretending for a moment that Optional is nothing more than a just another generic enum, we see that we simply have to write:

numbersB[safe: 2] = Optional(.none)

But this just serves to show how confusing this would be, especially since people seems to have such a hard time wrapping their head around nested optionals (but not eg nested arrays?).


#59

Well, putting the culprit on the call side is not really a solution, right? ;)


(Jens Persson) #60

You mean you'd like an optional-array-accessor-subscript-thing that works kind of like Dictionary does when setting a previously existing value to nil here:

var d = [Int : Int]()
d[42] = 123
print(d[42]) // Optional(123)
d[42] = nil
print(d[42]) // nil

var od = [Int : Int?]()
od[42] = 123
print(od[42]) // Optional(Optional(123))
od[42] = nil
print(od[42]) // nil

?

If so, I'd say use a Dictionary (with Int keys). Or write a proposal to add some more special case compiler magic to optionals in order to be able to implement it in a non-horrible way (but cause further confusion about (perhaps especially nested) optionals), or do something horrible like:

public protocol OptionalProtocol {
    associatedtype Wrapped
    var value: Optional<Wrapped> { get }
    mutating func setToNil()
}

extension Optional : OptionalProtocol {
    public var value: Optional<Wrapped> { return self }
    public mutating func setToNil() { self = .none }
}

extension Array {
    public subscript(hmm index: Index) -> Element? {
        get { return indices.contains(index) ? self[index] : nil }
        set (newValue) {
            if let v = newValue, indices.contains(index) { self[index] = v }
        }
    }
}

extension Array where Element: OptionalProtocol {
    public subscript(hmm index: Index) -> Element? {
        get { return indices.contains(index) ? self[index] : nil }
        set {
            guard indices.contains(index) else { return }
            if let v = newValue {
                self[index] = v
            } else {
                switch newValue.value {
                case .some(let v): self[index] = v
                case .none: self[index].setToNil()
                }
            }
        }
    }
}

(Karl) #61

I don't consider this "fine" or "as expected" at all. I think it is totally unacceptable for the setter to silently fail.

I said already that I'm not even convinced this is worth adding, but at least something which only reads is clearly understandable and doesn't really do any harm. However, I'm absolutely, strenuously, ∞% against having a setter which just ignores you and continues to return nil if you attempt to set an out-of-bounds index. That is just not appropriate for the standard library.