This makes it so that if you call removeLast() on an empty collection you get a fatal error.
("fatal error: can't remove last element from an empty collection")
Surely you could just remove the explicit unwrapping of 'last' and add a guard statement?
As such: @discardableResult
public mutating func removeLast() -> Iterator.Element? {
guard let element = last else {
return nil
}
self = self[startIndex..<index(before: endIndex)]
return element
}
It sure seems more "Swifty" to alert at compile time that removing the last item of a collection might fail, and make it return nil as a result.
This makes it so that if you call removeLast() on an empty collection you get a fatal error.
("fatal error: can't remove last element from an empty collection")
Surely you could just remove the explicit unwrapping of 'last' and add a guard statement?
As such: @discardableResult
public mutating func removeLast() -> Iterator.Element? {
guard let element = last else {
return nil
}
self = self[startIndex..<index(before: endIndex)]
return element
}
It sure seems more "Swifty" to alert at compile time that removing the last item of a collection might fail, and make it return nil as a result.
This makes it so that if you call removeLast() on an empty collection you get a fatal error.
("fatal error: can't remove last element from an empty collection")
Surely you could just remove the explicit unwrapping of 'last' and add a guard statement?
As such: @discardableResult
public mutating func removeLast() -> Iterator.Element? {
guard let element = last else {
return nil
}
self = self[startIndex..<index(before: endIndex)]
return element
}
It sure seems more "Swifty" to alert at compile time that removing the last item of a collection
might fail, and make it return nil as a result.
I see, the really issue I'm trying to bring up is that removeLast() can easily fail (and result in a fatal error).
And unless you read the documentation, that's not very obvious.
Maybe another approach would be to make removeLast() throw an exception if the collection is empty?
That way you can use "try" to safely remove the last item of a collection.
The reasoning behind this is that right now, to safely use removeLast(), you pretty much have to do something like this:
if !collection.isEmpty {
collection.removeLast()
}
Having this method throw would allow to change the above to:
try collection.removeLast()
– Louis D'hauwe
···
On 17 Oct 2016, at 22:52, Max Moiseev <moiseev@apple.com> wrote:
This makes it so that if you call removeLast() on an empty collection you get a fatal error.
("fatal error: can't remove last element from an empty collection")
Surely you could just remove the explicit unwrapping of 'last' and add a guard statement?
As such: @discardableResult
public mutating func removeLast() -> Iterator.Element? {
guard let element = last else {
return nil
}
self = self[startIndex..<index(before: endIndex)]
return element
}
It sure seems more "Swifty" to alert at compile time that removing the last item of a collection might fail, and make it return nil as a result.
I believe, sometimes there are situations where you know for sure that your collection is not empty. Maybe you are already in the context where the check has been performed. In these cases there is no reason you’d have to pay the price of an emptiness check once again.
Perhaps, it should be documented better, as in “popLast should generally be preferred since it is safer to use. You should only opt for the removeLast if you are certain the collection is not empty and performance is an issue”.
If you agree, mind sending us a pull request with such a documentation change?
Thanks!
Max
···
On Oct 17, 2016, at 2:19 PM, Louis D'hauwe <louisdhauwe@silverfox.be> wrote:
I see, the really issue I'm trying to bring up is that removeLast() can easily fail (and result in a fatal error).
And unless you read the documentation, that's not very obvious.
Maybe another approach would be to make removeLast() throw an exception if the collection is empty?
That way you can use "try" to safely remove the last item of a collection.
The reasoning behind this is that right now, to safely use removeLast(), you pretty much have to do something like this:
if !collection.isEmpty {
collection.removeLast()
}
Having this method throw would allow to change the above to:
try collection.removeLast()
– Louis D'hauwe
On 17 Oct 2016, at 22:52, Max Moiseev <moiseev@apple.com <mailto:moiseev@apple.com>> wrote:
This makes it so that if you call removeLast() on an empty collection you get a fatal error.
("fatal error: can't remove last element from an empty collection")
Surely you could just remove the explicit unwrapping of 'last' and add a guard statement?
As such: @discardableResult
public mutating func removeLast() -> Iterator.Element? {
guard let element = last else {
return nil
}
self = self[startIndex..<index(before: endIndex)]
return element
}
It sure seems more "Swifty" to alert at compile time that removing the last item of a collection might fail, and make it return nil as a result.
On 17 Oct 2016, at 23:20, Max Moiseev <moiseev@apple.com> wrote:
Hi Louis,
I believe, sometimes there are situations where you know for sure that your collection is not empty. Maybe you are already in the context where the check has been performed. In these cases there is no reason you’d have to pay the price of an emptiness check once again.
Perhaps, it should be documented better, as in “popLast should generally be preferred since it is safer to use. You should only opt for the removeLast if you are certain the collection is not empty and performance is an issue”.
If you agree, mind sending us a pull request with such a documentation change?
Thanks!
Max
On Oct 17, 2016, at 2:19 PM, Louis D'hauwe <louisdhauwe@silverfox.be <mailto:louisdhauwe@silverfox.be>> wrote:
I see, the really issue I'm trying to bring up is that removeLast() can easily fail (and result in a fatal error).
And unless you read the documentation, that's not very obvious.
Maybe another approach would be to make removeLast() throw an exception if the collection is empty?
That way you can use "try" to safely remove the last item of a collection.
The reasoning behind this is that right now, to safely use removeLast(), you pretty much have to do something like this:
if !collection.isEmpty {
collection.removeLast()
}
Having this method throw would allow to change the above to:
try collection.removeLast()
– Louis D'hauwe
On 17 Oct 2016, at 22:52, Max Moiseev <moiseev@apple.com <mailto:moiseev@apple.com>> wrote:
This makes it so that if you call removeLast() on an empty collection you get a fatal error.
("fatal error: can't remove last element from an empty collection")
Surely you could just remove the explicit unwrapping of 'last' and add a guard statement?
As such: @discardableResult
public mutating func removeLast() -> Iterator.Element? {
guard let element = last else {
return nil
}
self = self[startIndex..<index(before: endIndex)]
return element
}
It sure seems more "Swifty" to alert at compile time that removing the last item of a collection might fail, and make it return nil as a result.
You have to pay the price anyway, as the check has to be performed to decide if the software should abort.
···
Le 17 oct. 2016 à 23:20, Max Moiseev via swift-evolution <swift-evolution@swift.org> a écrit :
Hi Louis,
I believe, sometimes there are situations where you know for sure that your collection is not empty. Maybe you are already in the context where the check has been performed. In these cases there is no reason you’d have to pay the price of an emptiness check once again.
Yes, if the author of the collection you’re using performs the check in `removeLast`, but they don’t have to.
···
On Oct 18, 2016, at 1:28 PM, Jean-Daniel <dev@xenonium.com> wrote:
Le 17 oct. 2016 à 23:20, Max Moiseev via swift-evolution <swift-evolution@swift.org> a écrit :
Hi Louis,
I believe, sometimes there are situations where you know for sure that your collection is not empty. Maybe you are already in the context where the check has been performed. In these cases there is no reason you’d have to pay the price of an emptiness check once again.
You have to pay the price anyway, as the check has to be performed to decide if the software should abort.
I’m fairly confident the author of the collection has to make those checks for memory-safety, but in theory there’s wins in only doing the check once, and as early as possible. Smaller values to pass, and less checks.
This is definitely micro-micro-optimization, though. Unlikely to matter for most cases.
···
On Oct 18, 2016, at 6:00 PM, Max Moiseev via swift-evolution <swift-evolution@swift.org> wrote:
Yes, if the author of the collection you’re using performs the check in `removeLast`, but they don’t have to.
On Oct 18, 2016, at 1:28 PM, Jean-Daniel <dev@xenonium.com> wrote:
Le 17 oct. 2016 à 23:20, Max Moiseev via swift-evolution <swift-evolution@swift.org> a écrit :
Hi Louis,
I believe, sometimes there are situations where you know for sure that your collection is not empty. Maybe you are already in the context where the check has been performed. In these cases there is no reason you’d have to pay the price of an emptiness check once again.
You have to pay the price anyway, as the check has to be performed to decide if the software should abort.