Fix Deque.reserve. #2943
Fix Deque.reserve. #2943
Conversation
lib/core/collections.toit
Outdated
| // performance, we can reimplement with the a more traditional | ||
| // implementation. | ||
| first_ := 0 | ||
| last_ := 0 |
There was a problem hiding this comment.
Nit: It is a little bit weird that first is inclusive and last isn't, just from a naming perspective. Maybe start/end would be nicer?
lib/core/collections.toit
Outdated
| result = backing.remove-last | ||
| else: | ||
| result = backing[last - 1] | ||
| last_ = last - 1 |
There was a problem hiding this comment.
You should probably null out the removed element in the backing store, like it is done for remove-first.
| Returns the value that was removed. | ||
| */ | ||
| remove --at/int -> any: | ||
| last := size - 1 |
There was a problem hiding this comment.
This is a different last than last_.
| if at >= last >> 1: | ||
| return backing_.remove --at=(first_ + at) | ||
| if not 0 <= at <= last: throw "OUT_OF_BOUNDS" | ||
| if at >= (last >> 1): |
There was a problem hiding this comment.
done. And refactored it to make it easier to follow.
lib/core/collections.toit
Outdated
| last := last_ | ||
| reserved := backing.size - last | ||
| sz := last - first | ||
| if first * 2 > last: |
There was a problem hiding this comment.
Add a comment here? Why do we multiply here and shift in the other place?
There was a problem hiding this comment.
switched to shift for consistency.
done.
lib/core/collections.toit
Outdated
| backing.resize size | ||
| last := last_ | ||
| reserved := backing.size - last | ||
| sz := last - first |
There was a problem hiding this comment.
Why do we compute this eagerly? Why isn't this called size?
There was a problem hiding this comment.
somehow this file started to copy fields into locals (probably for efficiency reasons), and I have continued that "tradition".
It can't be called "size", as that would shadow the method, which isn't allowed.
tests/deque-test.toit
Outdated
| expect-equals "[1, 42]" d.stringify | ||
| expect-equals "[103, 3]" d2.stringify | ||
|
|
||
|
|
No description provided.