Collection initializer with an expression. #537
Conversation
Generated by 🚫 Danger |
Codecov Report
@@ Coverage Diff @@
## master #537 +/- ##
=========================================
+ Coverage 93.89% 93.9% +0.01%
=========================================
Files 66 66
Lines 2800 2806 +6
=========================================
+ Hits 2629 2635 +6
Misses 171 171
Continue to review full report at Codecov.
|
| @@ -11,6 +11,13 @@ import XCTest | |||
|
|
|||
| class RangeReplaceableCollectionTests: XCTestCase { | |||
|
|
|||
| func testInitExpressionOfSize() { | |||
There was a problem hiding this comment.
I'm missing the test for condition the size negative or 0 within the test. Could you add this?
| /// - Parameters: | ||
| /// - expression: The expression to execute for each position of the collection. | ||
| /// - size: The size of the collection. | ||
| public init(expression: @autoclosure () -> Element, size: Int) { |
There was a problem hiding this comment.
Fascinating extension. My only comment is that I think size is not the right name. size does not mean the number of times you're executing something. count makes more sense here.
I would love it if you added a complementary constructor that I feel is sorely missing from Array (or RangeReplaceableCollection, which is very similar. That is:
init(value: Element, count: Int)which basically does the same thing as this method except it just calls append(value) instead of executing the expression.
I don't mind adding this myself, it just seems to go hand-in-hand with this one.
There was a problem hiding this comment.
@guykogus Such a constructor already exists on the standard lib called init(repeating:count) and in fact it a use case of that was the reason for me to have to create this one :))
Here's my use case bellow:
There was a problem hiding this comment.
Ugh, how did I miss that?! 🤦♂️
| @@ -11,6 +11,13 @@ import XCTest | |||
|
|
|||
| class RangeReplaceableCollectionTests: XCTestCase { | |||
|
|
|||
| func testInitExpressionOfSize() { | |||
| var array = [1, 2, 3] | |||
| let newArray = [Int](expression: array.removeLast(), size: array.count) | |||
There was a problem hiding this comment.
What happens if you do it for array.count + 1? Will it throw an error? Or crash?
There was a problem hiding this comment.
Crash, but I believe it's something to user handle. We can't control what is inside the expression :))
| /// - Parameters: | ||
| /// - expression: The expression to execute for each position of the collection. | ||
| /// - size: The size of the collection. | ||
| public init(expression: @autoclosure () -> Element, size: Int) { |
There was a problem hiding this comment.
What happens if it's a throwing closure? Should we mark it as throws and the function as rethrows? I believe that this is pretty standard practice (e.g. removeFirst). That would make the signature:
public init(expression: @autoclosure () throws -> Element, size: Int) rethrows
An convenience init to create a collection of a given size initialized with the values of an expression :))
Checklist