Negative index in array.remove off by one error?

[narimiran]

Is this intended? (Because definitely it isn't expected)

(array/remove @[10 20 30 40 50] -1) # doesn't remove anything
(array/remove @[10 20 30 40 50] -2) # removes last element

This + 1 in array.c looks suspicious:

janet/src/core/array.c

Line 300 in 3a4d56a

at = array->count + at + 1;

[czkz]

Similar behavior with slice:

(slice [10 20 30 40 50] -1) # => ()
(slice [10 20 30 40 50] -2) # => (50)

[pepe]

Check the doc here https://janet-lang.org/api/array.html#array/slice please

[sogaiu]

I think what @pepe might have been hinting at is that this is expected behavior. The negative index convention in Janet might be different from what one might expect from elsewhere but glancing at what has been posted above, they look consistent with what I'm used to.

See here for some examples.

[narimiran]

Check the doc here https://janet-lang.org/api/array.html#array/slice please

Ok, it seems it is intended. But then it should be mentioned and emphasized in the documentation of every function that uses that convention, because Janet is the only language I know to use -2 for the last element, and I'm sure I'm not the only one who was surprised with it.

[primo-ppcg]

I'm going to throw my hat in the ring, and say that the behavior of array/remove is probably wrong, or at very least doesn't follow the principle of least surprise.

On the other hand, I think that the behavior of array/slice is correct, however the docstring could, and probably should be reworded:

- Note that index -1 is synonymous with index `(length arrtup)` to allow a full negative slice range.
+ Note that if the range is negative, it is taken as (start, end] to allow a full negative slice range.

This defines the same behavior, without the need to define index -1 to be "outside" the array, which doesn't make sense for any other function.

[narimiran]

Check the doc here https://janet-lang.org/api/array.html#array/slice please

Now that I thought about it a bit more — I don't buy it.

Slice can be used without end, so there's no need for this "off by one" so that -1 is the length of the array and not the last element (which most of people/newcomers would expect).

(array/slice @[10 20 30 40 50] 1)
(array/slice @[10 20 30 40 50] -2)

No need to do:

(array/slice @[10 20 30 40 50] 1 -1)
(array/slice @[10 20 30 40 50] -2 -1)

[pepe]

@narimiran, you do not need to, indeed. But it is like that, and I do not think it will be changed.

[pepe]

Just some food for thought: you are not always writing number literals by hand as arguments of the functions :-).

[narimiran]

Just some food for thought: you are not always writing number literals by hand as arguments of the functions :-).

I don't see how that changes the fact that using -2 for the last element is, to put it mildly, surprising.

[pepe]

There are only two challenging problems in computer science: naming, cache invalidation, and +-1 problem :-).

[sogaiu]

@narimiran Do you think #1224 addressed this issue?

If so, may be this issue can be closed.

[narimiran]

Do you think #1224 addressed this issue?

Yep. Closing this.

Thanks @primo-ppcg!