From 70ee822f1962feab6328be79afe2f9db3b855b1b Mon Sep 17 00:00:00 2001
From: Scott Kyle <scott@appden.com>
Date: Mon, 28 Sep 2015 16:00:24 -0700
Subject: [PATCH] Update splice method to mimic JS standard

It now will return the removed elements, and clamps the first two arguments to be acceptable values rather than throwing an exception.
---
 src/RJSArray.cpp    | 18 +++++++++++-------
 tests/ArrayTests.js | 36 ++++++++++++++++++++++--------------
 2 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/src/RJSArray.cpp b/src/RJSArray.cpp
index bf6e975b..2681de9f 100644
--- a/src/RJSArray.cpp
+++ b/src/RJSArray.cpp
@@ -164,24 +164,28 @@ JSValueRef ArrayShift(JSContextRef ctx, JSObjectRef function, JSObjectRef thisOb
 JSValueRef ArraySplice(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* jsException) {
     try {
         ObjectArray *array = RJSVerifiedArray(thisObject);
+        size_t size = array->size();
+
         RJSValidateArgumentCountIsAtLeast(argumentCount, 2);
-        long index = RJSValidatedValueToNumber(ctx, arguments[0]);
+        long index = std::min<long>(RJSValidatedValueToNumber(ctx, arguments[0]), size);
         if (index < 0) {
-            index = array->size() + index;
+            index = std::max<long>(size + index, 0);
         }
 
-        long remove = RJSValidatedValueToNumber(ctx, arguments[1]);
-        if (index + remove > array->size()) {
-            throw std::runtime_error("Attempting to slice elements beyond Array bounds.");
+        long remove = std::max<long>(RJSValidatedValueToNumber(ctx, arguments[1]), 0);
+        if (index + remove > size) {
+            remove = size - index;
         }
 
-        while (remove-- > 0) {
+        std::vector<JSObjectRef> removedObjects(remove);
+        for (size_t i = 0; i < remove; i++) {
+            removedObjects[i] = RJSObjectCreate(ctx, Object(array->realm, array->object_schema, array->get(index)));
             array->link_view->remove(index);
         }
         for (size_t i = 2; i < argumentCount; i++) {
             array->link_view->insert(index + i - 2, RJSAccessor::to_object_index(ctx, array->realm, const_cast<JSValueRef &>(arguments[i]), array->object_schema.name, false));
         }
-        return JSValueMakeNumber(ctx, array->link_view->size());
+        return JSObjectMakeArray(ctx, remove, removedObjects.data(), jsException);
     }
     catch (std::exception &exp) {
         if (jsException) {
diff --git a/tests/ArrayTests.js b/tests/ArrayTests.js
index 53372df6..c07a3d7d 100644
--- a/tests/ArrayTests.js
+++ b/tests/ArrayTests.js
@@ -205,40 +205,48 @@ var ArrayTests = {
 
     testSplice: function() {
         var realm = new Realm({schema: [LinkTypesObjectSchema, TestObjectSchema]});
-        var array;
+
         realm.write(function() {
             var obj = realm.create('LinkTypesObject', [[1], [2], [[3], [4]]]);
-            array = obj.arrayCol;
+            var array = obj.arrayCol;
+            var removed;
 
-            TestCase.assertEqual(array.splice(0, 0, obj.objectCol, obj.objectCol1), 4);
+            removed = array.splice(0, 0, obj.objectCol, obj.objectCol1);
+            TestCase.assertEqual(removed.length, 0);
             TestCase.assertEqual(array.length, 4);
             TestCase.assertEqual(array[0].doubleCol, 1);
             TestCase.assertEqual(array[1].doubleCol, 2);
 
-            TestCase.assertEqual(array.splice(2, 2, [5], [6]), 4);
+            removed = array.splice(2, 2, [5], [6]);
+            TestCase.assertEqual(removed.length, 2);
+            TestCase.assertEqual(removed[0].doubleCol, 3);
+            TestCase.assertEqual(removed[1].doubleCol, 4);
             TestCase.assertEqual(array.length, 4);
             TestCase.assertEqual(array[2].doubleCol, 5);
             TestCase.assertEqual(array[3].doubleCol, 6);
 
-            TestCase.assertEqual(array.splice(2, 2), 2);
+            removed = array.splice(2, 2);
+            TestCase.assertEqual(removed.length, 2);
+            TestCase.assertEqual(removed[0].doubleCol, 5);
+            TestCase.assertEqual(removed[1].doubleCol, 6);
+            TestCase.assertEqual(array.length, 2);
             TestCase.assertEqual(array[0].doubleCol, 1);
             TestCase.assertEqual(array[1].doubleCol, 2);
-            TestCase.assertEqual(array.length, 2);
 
-            TestCase.assertEqual(array.splice(-1, 1), 1);
+            removed = array.splice(-1, 1);
+            TestCase.assertEqual(removed.length, 1);
+            TestCase.assertEqual(removed[0].doubleCol, 2);
             TestCase.assertEqual(array.length, 1);
             TestCase.assertEqual(array[0].doubleCol, 1);
 
-            TestCase.assertThrows(function() {
-                array.splice(0, 2);
-            });
+            removed = array.splice(0, 2);
+            TestCase.assertEqual(removed.length, 1);
+            TestCase.assertEqual(removed[0].doubleCol, 1);
+            TestCase.assertEqual(array.length, 0);
+            
             TestCase.assertThrows(function() {
                 array.splice(0, 0, 0);
             });
         });
-
-       // TestCase.assertThrows(function() {
-       //      array.shift();
-       //  });
     },
 };