hamstergem / hamster

Efficient, Immutable, Thread-Safe Collection classes for Ruby
Other
1.89k stars 94 forks source link

Vector#set fails beyond end of vector #160

Closed dubek closed 10 years ago

dubek commented 10 years ago

With Ruby Array we have:

a = []
a[0] = "foo"
a
# => ["foo"]

a = ["x", "y"]
a[4] = "foo"
a
# => ["x", "y", nil, nil, "foo"]

However, Vector doesn't allow #set on empty vector (even at position 0):

Hamster::Vector[].set(0, "foo")
# => IndexError: IndexError

but allow extending the vector by one (if it wasn't empty):

Hamster::Vector["x", "y"].set(2, "foo")
# => Hamster::Vector["x", "y", "foo"]

but doesn't allow setting beyond the last + 1 element:

Hamster::Vector["x", "y"].set(4, "foo")
# => IndexError: IndexError

(even if we leave it we can add some description to the exception like we have in #fetch.)

alexdowad commented 10 years ago

The fact that a Vector can be extended by only 1 was deliberate; I did it that way simply because it was easier to implement. If you want to improve this, that would be great.

The point about how an empty Vector can't be extended is a bug, and needs to be fixed.

alexdowad commented 10 years ago

Pushed a fix with e25f564. It is still only possible to extend by 1, feel free to fix that if you like.

dubek commented 10 years ago

I'm considering calling #fill with nil for all the "missing" indexes, if needed. I'll try it out.

dubek commented 10 years ago

BTW the reason we didn't find this bug is that in vector/get_spec.rb we have if rand(1) == 0 but rand(1) always returns 0; it should be rand(2). Once you change that, you start to get IndexError exceptions...

dubek commented 10 years ago

Commit 00fc44189f99993ac131cc19351ec23cd23fc589 on master allows to set element past the end of the vector:

Hamster::Vector[1,2,3].set(6, "foo")
# => Hamster::Vector[1, 2, 3, nil, nil, nil, "foo"]