-
Notifications
You must be signed in to change notification settings - Fork 323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement Vector.build
: implementation and some tests
#9725
Changes from 9 commits
c07f260
4bd580e
4194d15
7b4128a
e28c006
4de1bbc
ae3110a
71219a2
36a7023
7cf7649
81d8556
8b7b1f3
ec44244
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -91,11 +91,13 @@ type Vector a | |||||
Vector.collect (List.Cons 1 <| List.Cons 2 <| List.Nil) .x .xs stop_at=(_==List.Nil) | ||||||
collect : Any -> (Any -> Any) -> (Any -> Any) -> Integer | Nothing -> (Any -> Boolean) -> Vector Any | ||||||
collect seq element:(Any -> Any) next:(Any -> Any) limit:(Integer | Nothing)=Nothing stop_at:(Any -> Boolean)=(_==Nothing) = | ||||||
b = Vector.new_builder (if limit.is_nothing then 10 else limit) | ||||||
iterate item remaining = if remaining == 0 || (stop_at item) then b.to_vector else | ||||||
b.append <| element item | ||||||
@Tail_Call iterate (next item) (if remaining.is_nothing then Nothing else remaining-1) | ||||||
iterate seq limit | ||||||
Vector.build capacity=(if limit.is_nothing then 10 else limit) builder-> | ||||||
iterate item remaining = | ||||||
done = remaining == 0 || (stop_at item) | ||||||
if done.not then | ||||||
builder.append <| element item | ||||||
@Tail_Call iterate (next item) (if remaining.is_nothing then Nothing else remaining-1) | ||||||
iterate seq limit | ||||||
|
||||||
## PRIVATE | ||||||
ADVANCED | ||||||
|
@@ -145,14 +147,15 @@ type Vector a | |||||
|
||||||
## PRIVATE | ||||||
ADVANCED | ||||||
DEPRECATED | ||||||
Creates a new vector builder instance. | ||||||
|
||||||
A vector builder is a mutable data structure, that allows for gathering | ||||||
A vector `Builder` is a mutable data structure, that allows for gathering | ||||||
a number of elements and then converting them into a vector. This is | ||||||
particularly useful when the number of elements is not known upfront. | ||||||
|
||||||
A vector allows to store an arbitrary number of elements in linear memory. It | ||||||
is the recommended data structure for most applications. | ||||||
A vector allows to store an arbitrary number of elements in linear | ||||||
memory. It is the recommended data structure for most applications. | ||||||
|
||||||
Arguments: | ||||||
- capacity: Initial capacity of the Vector.Builder | ||||||
|
@@ -171,6 +174,50 @@ type Vector a | |||||
new_builder : Integer -> Builder | ||||||
new_builder (capacity=10) = Builder.new capacity | ||||||
|
||||||
## PRIVATE | ||||||
ADVANCED | ||||||
Creates a new `Vector` by providing a `Builder` to the provided function. | ||||||
|
||||||
A vector `Builder` is a mutable data structure, that allows for gathering | ||||||
a number of elements and then converting them into a vector. This is | ||||||
particularly useful when the number of elements is not known upfront. | ||||||
|
||||||
`.build` creates a new `Builder`, passes it to the provided function, | ||||||
which can add elements using the `Builder`'s `.append` method. When the | ||||||
function is done, `.build` then closes the `Builder and returns the | ||||||
resulting `Vector`. | ||||||
|
||||||
The provided function should call `.append` to add new elements to the | ||||||
`Builder`. The return value of the provided function is not used, unless | ||||||
it is a dataflow error, in which case the `Vector` is not built, and the | ||||||
dataflow error is propagted instead. | ||||||
|
||||||
A vector allows to store an arbitrary number of elements in linear | ||||||
memory. It is the recommended data structure for most applications. | ||||||
|
||||||
Arguments: | ||||||
- function: a function taking a `Builder` and using it to add elements. | ||||||
- capacity: Initial capacity of the Vector.Builder | ||||||
|
||||||
! Error Conditions | ||||||
|
||||||
- If the provided function throws a dataflow error, the `Vector` is not | ||||||
built, and the error is propagted instead. | ||||||
|
||||||
> Example | ||||||
Construct a vector using a builder that contains the items 1 to 5. | ||||||
|
||||||
Vector.build builder-> | ||||||
builder.append 1 | ||||||
builder.append 2 | ||||||
builder.append 3 | ||||||
# => [1, 2, 3] | ||||||
build : (Builder -> Any) -> Integer -> Vector | ||||||
build (function : Builder -> Any) (capacity : Integer = 10) -> Vector = | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I think maybe adding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||
builder = Builder.new capacity | ||||||
result = function builder | ||||||
result.if_not_error builder.to_vector | ||||||
|
||||||
## PRIVATE | ||||||
ADVANCED | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -420,27 +420,25 @@ type_spec suite_builder name alter = suite_builder.group name group_builder-> | |
|
||
group_builder.specify "should allow applying a function to each element" <| | ||
vec = alter [1, 2, 3, 4] | ||
vec_mut = Vector.new_builder | ||
vec.each vec_mut.append | ||
vec_mut.to_vector . should_equal vec | ||
result = Vector.build builder-> | ||
vec.each builder.append | ||
result . should_equal vec | ||
|
||
group_builder.specify "should accept changed elements" <| | ||
vec_mut = Vector.new_builder | ||
vec_mut.append 1 | ||
vec_mut.append 1.1 | ||
vec_mut.append Nothing | ||
vec = alter <| Vector.build builder-> | ||
builder.append 1 | ||
builder.append 1.1 | ||
builder.append Nothing | ||
|
||
vec = alter vec_mut.to_vector | ||
vec.length . should_equal 3 | ||
vec.at 0 . should_equal 1 | ||
vec.at 1 . should_equal 1.1 | ||
vec.at 2 . should_equal Nothing | ||
|
||
group_builder.specify "should accept Nothing" <| | ||
vec_mut = Vector.new_builder | ||
vec_mut.append Nothing | ||
vec = alter <| Vector.build builder-> | ||
builder.append Nothing | ||
|
||
vec = alter vec_mut.to_vector | ||
vec.length . should_equal 1 | ||
vec.at 0 . should_equal Nothing | ||
|
||
|
@@ -778,21 +776,19 @@ type_spec suite_builder name alter = suite_builder.group name group_builder-> | |
|
||
group_builder.specify "should correctly propagate state through each" <| | ||
v = State.run Number 77 <| | ||
b = Vector.new_builder | ||
(alter ["A", "B"]).each x-> | ||
b.append x | ||
b.append (State.get Number).to_text | ||
b.to_vector | ||
Vector.build builder-> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd assume this is a general even for complicated computations. Just wrap your computation in |
||
(alter ["A", "B"]).each x-> | ||
builder.append x | ||
builder.append (State.get Number).to_text | ||
|
||
v.should_equal ['A', '77', 'B', '77'] | ||
|
||
group_builder.specify "should correctly propagate state through map" <| | ||
v = State.run Number 55 <| | ||
b = Vector.new_builder | ||
(alter ["X", "Y"]).map x-> | ||
b.append x | ||
b.append (State.get Number).to_text | ||
b.to_vector | ||
Vector.build builder-> | ||
(alter ["X", "Y"]).map x-> | ||
builder.append x | ||
builder.append (State.get Number).to_text | ||
|
||
v.should_equal ['X', '55', 'Y', '55'] | ||
|
||
|
@@ -1034,6 +1030,26 @@ add_specs suite_builder = | |
v = Vector.collect l .x .xs limit=30 stop_at=(_==List.Nil) | ||
v . should_equal [1, 2, 3] | ||
|
||
suite_builder.group "Vector.build" group_builder-> | ||
group_builder.specify "Should be able to build a vector" <| | ||
v = Vector.build builder-> | ||
builder.append 1 | ||
builder.append 2 | ||
builder.append Nothing | ||
builder.append 3 | ||
v . should_equal [1, 2, Nothing, 3] | ||
|
||
v2 = Vector.build builder-> | ||
_ = builder # Do nothing | ||
v2 . should_equal [] | ||
|
||
group_builder.specify "Should propagate dataflow errors" <| | ||
v = Vector.build builder-> | ||
builder.append 1 | ||
builder.append 2 | ||
Error.throw (Illegal_Argument.Error "asdf") | ||
v . should_fail_with (Illegal_Argument.Error "asdf") | ||
Comment on lines
+1050
to
+1055
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a great test. Not directly related, but while we are at it we could consider solving a long outstanding problem we used to have:
Unfortunately, as far as I know currently this will actually not fail but produce a vector We used to have a pattern of 'folding' the builder i.e.
in which case the error would be correctly propagated. Unfortunately, the 'folding the builder' pattern did not get much traction. In many places it was just not too practical - the builder was inherently mutable object so this more 'functional' folding was often not fitting its usages. Do you think we could amend the builder to detect that This could be implemented by holding a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But this is a bit out of scope of this PR, so I guess we can also do it as a separate ticket/PR instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very good idea. #9733 |
||
|
||
suite_builder.group "Vector/Array equality" group_builder-> | ||
v1 = [1, 2, 3] | ||
a1 = v1.to_array | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rephrase this sentence as providing X to the provided Y sounds weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to "passing a
Builder
to the provided function"