[Draft] Adding Safe indexing to Array

Isn't Collection.Index: Comparable? Also is accessing endIndex O(1)?
If both are yes, then the check is O(1) as well. If not, this functionality can be restricted to the wider collection type that satisfies them (BidirectionalCollection ?? RandomAccessCollection) as any of them will still include Array.

3 Likes

What is the use case for indexing arrays with unbounded indexes? Where are these indexes coming from if they aren't bounded by something like 0..<array.count? If they are from another array, the user should just bound iteration to the smaller one's count.

The only valid case I see is array[0] for which we have first (and even last) which already returns an Optional.

This could also (depending on the naming) lead unexperienced programmers to think that this is the preferred way to use arrays since it's "safe" or "checked".

2 Likes

To me, a[safe: i] reads more as "this index is safe to use" than "this index was confirmed to be safe beforehand", whereas valid, existing and checked all read as the inverse. I think it ends up being a subjective thing whether it reads one way or the other.

Xiaodi's suggestion of ifExists is the most unambiguous to me, although something about it doesn't sit right with me. safe is definitely the most natural option for me personally, though I think the case against it is reasonable.

Yes and yes, however the check you are referring to, (startIndex..<endIndex).contains(i), is different from indices.contains(i). Only RandomAccessCollection guarantees that all values between startIndex and endIndex are valid indices.

2 Likes

It's a very clean way to avoid bounds checking in any situation where an index is not guaranteed to be valid. I looked at the use cases for my own safe subscripting extension, and the most common reason I use it is for retrieving an adjacent element in an array. It's nice to be able to do if let next = array[safe: index + 1] to get the next element without going past the end.

I don't share the concern that users will prefer this API by default. I think the extra syntax via the label and being forced to deal with the optionality are enough to discourage its use in situations where it's not needed.

2 Likes

I hadn't thought of this use case, which is compelling. It would be good to include a few examples like this in the proposal

3 Likes

I don't like this idea. For me, trying to access an array element (or collection for that matter) with an out of bound index is a programmer error and should crash.

In rare cases and algorithms that this might make sense and end up being more readable, I may add a private extension to Array or Collection to take care of it. It is just a few lines of straightforward hard to screw-up code. I don't think such helper facility belongs in the standard library.

1 Like

Okay, if we are *only* trying to do a bounds check, and not validate the index if it is within the bounds, then I think we can do this in an extension on Collection. (If we want it to be both fast and actually check that the index is valid, then it would go on RandomAccessCollection.)

Declaration:

extension Collection {
  subscript(optional idx: Index?) -> Element? {
    if let i = idx, (startIndex ..< endIndex).contains(i) { return self[i] }
    return nil
  }
}

Usage:

let anArray = ["I", "am", "Groot"]
let i = anArray.index{ $0.count > 5 }
let j = anArray.index{ $0.count > 3 }
let k = Optional(1)
let l = (j == k) ? nil : 0

anArray[optional: -1]  // nil
anArray[optional: 0]   // Optional("I")
anArray[optional: 1]   // Optional("am")
anArray[optional: 2]   // Optional("Groot")
anArray[optional: 3]   // nil
anArray[optional: i]   // nil
anArray[optional: j]   // Optional("Groot")
anArray[optional: k]   // Optional("am")
anArray[optional: l]   // Optional("I")
anArray[optional: nil] // nil

I share @hooman’s concerns though, and am not convinced that this, nor anything like it, need to be in the standard library.

2 Likes

When this comes up for me, it's situations where I want an optional anyway, so the bounds check can just be combined into the subscript. My extension uses ifPresent:. It's handy to have around, I'd like to see it added if just to standardise a good name.

But containsOnly seems more deserving of a proposal right now, as the logic to achieve that gets quite mind-bendy (!array.contains(where: {!check($0)})). Meanwhile ifPresent: just saves an explicit bounds check.

Huh, it is. I didn't realize this, since it's not that prominent in the documentation. This makes it a lot harder (if not impossible!) to write custom Swift Collections such as linked lists or search trees. If anyone has one that can satisfy Collection's guarantees, I'd love to see it!

I am strongly opposed to this. If we're going to add safe indexing, it should actually make sure that the index is valid, not perform a psuedo-validation that kinda sorta does a check.

I think that function for such purposes would be great. I think that there is a risk of doing subscripts a replacement for functions in case of collections. It is not bad but can lead to develop two independent collection usage approaches in future: via functions and via subscripts.
Function in this case is simply addition but not replacement. Because some unexperienced users may think that new subscript is just safe replacement of unsafe variant. They can start to use it everywhere or companies can start to forbid "old-style" unsafe subscript usage.

1 Like

To add on on this reply, I would say that a read-only subscript should be considered as a function first. However we should choose the name carefully so we don't end up in some sort of a collision in the future if the protocols like Collection would want to introduce a similar named function but with a different result.

1 Like

Maybe we should collect more usecases of a checked subscript because I think that there might be better solutions. For a single subscript call these new subscripts are great but for multiple ones there are almost always better solutions which are more performant.

For example:

  • Making pairs of adjacent elements: Loop over all indices i and use i + 1 with the new subscript. This introduces for all indices an additional check. But this is only needed for the last index, making the loop slower.

    I think a specialized (Lazy)Collection would be more suitable for this case.

  • Using the wrapped subscript: If you loop over adjacent indices (or even strided indices) it would introduce a slow %-Operation for every index. This would be better solved with a proper PeriodicCollection.

Just a thought: Maybe a new IndexSequence protocol could solve these problems more generally

1 Like

I appreciate the problem that this is trying to address. I don’t think an Array should return an Optional. I think negative indexes would be really cool. I would suggest merging these two features into a default subscript like dictionary already has.
https://developer.apple.com/documentation/swift/dictionary/2894528-subscript

I'd rather see both. I already use my own extensions for each, but in practice I only use "default" to set ( it's more like arr[37, fill:"foo"] ) because it's a bit verbose and confusing to get. It's a bit counterintuitive that arrays in Swift don't return optionals to begin with. I'm +1 for adding both [safe:Index] and [index:Index, default:Element] (or called fill:Element)

One use case for me is writing unit tests. I often check the results of some operation by checking all the values in the array in a sequence, like this:

	let pathString = "#f=FFF/g=GGG/u=UUU/___"
	let path = FragmentPath(fragment: pathString)
	XCTAssertEqual(path.selectors.count, 4)
	XCTAssertEqual(path.selectors.at(0), FragmentPath.Selector(prefix: .file, identifier: "FFF"))
	XCTAssertEqual(path.selectors.at(1), FragmentPath.Selector(prefix: .group, identifier: "GGG"))
	XCTAssertEqual(path.selectors.at(2), FragmentPath.Selector(prefix: .unit, identifier: "UUU"))
	XCTAssertEqual(path.selectors.at(3), FragmentPath.Selector(prefix: nil, identifier: "___"))

Here I use a custom Array.at(_:) as my "safe" indexing function. If for some reason count was less than 4 with normal indexing the test would crash when attempting to check the missing elements. Whereas with this I get a proper error report.

1 Like

Maybe something like

let array = [1, 2, 3]
array[safe: 4...]

I’m sorry, I don’t understand what you mean by this code snippet:

In order to clarify: I meant by

let array = [1, 2, 3, 4]
let adjacentPairs = [(1,2), (2,3), (3,4)]

I'm +1 on this proposal with the subscript name "checked:". As mentioned upthread, the subscript name "safe:" is problematic because the notion of safety here is confusing.

-Chris

I really like the optional name for the safe indexing subscript as it clearly states the output type.

1 Like