Repository: sanctuary-js/sanctuary
Pull request #622 (2019-04-05 15:57:51)

array: remove special treatment of negative offsets
Treating negative offsets specially is a source of bugs.

I recently wrote some code to access the element preceding an element at a given index. I was about to use `S.at (idx - 1) (xs)`, but realized that this would evaluate to `Just (xs[xs.length - 1])` when `idx` is `0`. Wrap-around behaviour is unwanted in such cases! I solved the problem by writing my own `at` function which always returns `Nothing` when given a negative index. `S.at` and `S.slice` should do the same.



Patch:
diff --git a/index.js b/index.js
index 4d9bcb7..97085f2 100644
--- a/index.js
+++ b/index.js
@@ -2849,7 +2849,6 @@
   //.
   //. Takes a start index `i`, an end index `j`, and an array, and returns
   //. Just the `[i,j)` slice of the array if possible; Nothing otherwise.
-  //. A negative index represents an offset from the length of the array.
   //.
   //. See also [`take`](#take), [`drop`](#drop), [`takeLast`](#takeLast),
   //. and [`dropLast`](#dropLast).
@@ -2858,23 +2857,15 @@
   //. > S.slice (1) (3) (['a', 'b', 'c', 'd', 'e'])
   //. Just (['b', 'c'])
   //.
-  //. > S.slice (-3) (-1) (['a', 'b', 'c', 'd', 'e'])
-  //. Just (['c', 'd'])
-  //.
   //. > S.slice (1) (6) (['a', 'b', 'c', 'd', 'e'])
   //. Nothing
   //. ```
-  function slice(start) {
-    return function(end) {
+  function slice(fromIdx) {
+    return function(toIdx) {
       return function(xs) {
-        var fromIdx = start < 0 ? start + xs.length : start;
-        var toIdx = end < 0 ? end + xs.length : end;
-
-        return Math.abs (start) <= xs.length &&
-               Math.abs (end) <= xs.length &&
-               fromIdx <= toIdx ?
-                 Just (xs.slice (fromIdx, toIdx)) :
-                 Nothing;
+        return fromIdx <= toIdx && fromIdx >= 0 && toIdx <= xs.length ?
+               Just (xs.slice (fromIdx, toIdx)) :
+               Nothing;
       };
     };
   }
@@ -2887,8 +2878,7 @@
   //# at :: Integer -> Array a -> Maybe a
   //.
   //. Returns Just the element of the given array at the specified index if
-  //. the index is within the array's bounds; Nothing otherwise. A negative
-  //. index represents an offset from the length of the array.
+  //. the index is within the array's bounds; Nothing otherwise.
   //.
   //. ```javascript
   //. > S.at (2) (['a', 'b', 'c', 'd', 'e'])
@@ -2896,13 +2886,9 @@
   //.
   //. > S.at (5) (['a', 'b', 'c', 'd', 'e'])
   //. Nothing
-  //.
-  //. > S.at (-2) (['a', 'b', 'c', 'd', 'e'])
-  //. Just ('d')
   //. ```
-  function at(n) {
+  function at(idx) {
     return function(xs) {
-      var idx = n < 0 ? xs.length + n : n;
       return idx < 0 || idx >= xs.length ? Nothing : Just (xs[idx]);
     };
   }


Test patch:
diff --git a/test/at.js b/test/at.js
index c8e6704..06800f6 100644
--- a/test/at.js
+++ b/test/at.js
@@ -12,9 +12,9 @@ test ('at', () => {
   eq (S.show (S.at)) ('at :: Integer -> Array a -> Maybe a');
 
   eq (S.at (-4) (['foo', 'bar', 'baz'])) (S.Nothing);
-  eq (S.at (-3) (['foo', 'bar', 'baz'])) (S.Just ('foo'));
-  eq (S.at (-2) (['foo', 'bar', 'baz'])) (S.Just ('bar'));
-  eq (S.at (-1) (['foo', 'bar', 'baz'])) (S.Just ('baz'));
+  eq (S.at (-3) (['foo', 'bar', 'baz'])) (S.Nothing);
+  eq (S.at (-2) (['foo', 'bar', 'baz'])) (S.Nothing);
+  eq (S.at (-1) (['foo', 'bar', 'baz'])) (S.Nothing);
 
   eq (S.at (0) (['foo', 'bar', 'baz'])) (S.Just ('foo'));
   eq (S.at (1) (['foo', 'bar', 'baz'])) (S.Just ('bar'));
diff --git a/test/slice.js b/test/slice.js
index 088e1fb..2891381 100644
--- a/test/slice.js
+++ b/test/slice.js
@@ -17,15 +17,25 @@ test ('slice', () => {
   eq (S.slice (-6) (1) ([1, 2, 3, 4, 5])) (S.Nothing);
 
   eq (S.slice (1) (1) ([1, 2, 3, 4, 5])) (S.Just ([]));
-  eq (S.slice (1) (-4) ([1, 2, 3, 4, 5])) (S.Just ([]));
-  eq (S.slice (-4) (1) ([1, 2, 3, 4, 5])) (S.Just ([]));
-  eq (S.slice (-4) (-4) ([1, 2, 3, 4, 5])) (S.Just ([]));
+  eq (S.slice (1) (-4) ([1, 2, 3, 4, 5])) (S.Nothing);
+  eq (S.slice (-4) (1) ([1, 2, 3, 4, 5])) (S.Nothing);
+  eq (S.slice (-4) (-4) ([1, 2, 3, 4, 5])) (S.Nothing);
   eq (S.slice (0) (0) ([])) (S.Just ([]));
 
   eq (S.slice (1) (3) ([1, 2, 3, 4, 5])) (S.Just ([2, 3]));
-  eq (S.slice (-3) (5) ([1, 2, 3, 4, 5])) (S.Just ([3, 4, 5]));
-  eq (S.slice (1) (-2) ([1, 2, 3, 4, 5])) (S.Just ([2, 3]));
-
+  eq (S.slice (-3) (5) ([1, 2, 3, 4, 5])) (S.Nothing);
+  eq (S.slice (1) (-2) ([1, 2, 3, 4, 5])) (S.Nothing);
+
+  eq (S.slice (0) (0) ([1, 2, 3, 4, 5])) (S.Just ([]));
+  eq (S.slice (0) (1) ([1, 2, 3, 4, 5])) (S.Just ([1]));
+  eq (S.slice (0) (2) ([1, 2, 3, 4, 5])) (S.Just ([1, 2]));
+  eq (S.slice (0) (3) ([1, 2, 3, 4, 5])) (S.Just ([1, 2, 3]));
+  eq (S.slice (0) (4) ([1, 2, 3, 4, 5])) (S.Just ([1, 2, 3, 4]));
   eq (S.slice (0) (5) ([1, 2, 3, 4, 5])) (S.Just ([1, 2, 3, 4, 5]));
+  eq (S.slice (1) (5) ([1, 2, 3, 4, 5])) (S.Just ([2, 3, 4, 5]));
+  eq (S.slice (2) (5) ([1, 2, 3, 4, 5])) (S.Just ([3, 4, 5]));
+  eq (S.slice (3) (5) ([1, 2, 3, 4, 5])) (S.Just ([4, 5]));
+  eq (S.slice (4) (5) ([1, 2, 3, 4, 5])) (S.Just ([5]));
+  eq (S.slice (5) (5) ([1, 2, 3, 4, 5])) (S.Just ([]));
 
 });