Problem
function slice(array, startingPoint, endingPoint){
if(arguments.length === 1) {
return array;
}else if(arguments.length === 2) {
// non-int value
var numberInString = startingPoint;
if(typeof(startingPoint) !== 'number'){
var checkNumber = Number(startingPoint);
if(checkNumber.toString() === 'NaN'){
return array;
}else{
numberInString = checkNumber;
}
}
var result = [];
if(numberInString < 0) {
var posStartPoint = array.length - Math.abs(numberInString);
var updateArr = [];
for(var i = posStartPoint; i < array.length; i++){
updateArr.push(array[i]);
}
return updateArr;
}else if( numberInString > array.length) {
return [];
}
for(var i = numberInString; i < array.length; i++){
result.push(array[i]);
}
return result;
}else{
var numberInStartPoint = startingPoint;
var numberInEndPoint = endingPoint;
var updateArr = [];
if(typeof(endingPoint) !== 'number' || typeof(startingPoint) !== 'number'){
if(typeof(endingPoint) !== 'number' && typeof(startingPoint) !== 'number'){
var checkStartNumber = Number(startingPoint);
var checkEndNumber = Number(endingPoint);
if( checkEndNumber.toString() === 'NaN' || checkStartNumber.toString() === 'NaN'){
return updateArr;
}else{
numberInStartPoint = checkStartNumber;
numberInEndPoint = checkEndNumber;
}
}
}
var posEndingPoint = numberInEndPoint;
var posStartPoint = numberInStartPoint;
if(posEndingPoint < 0 || numberInStartPoint < 0 ) {
if( posEndingPoint < 0 && numberInStartPoint < 0 ) {
posEndingPoint = array.length - Math.abs(posEndingPoint);
posStartPoint = array.length - Math.abs(numberInStartPoint);
}else if( numberInStartPoint < 0 ) {
posStartPoint = array.length - Math.abs(numberInStartPoint);
}else{
posEndingPoint = array.length - Math.abs(posEndingPoint);
}
}else if( numberInStartPoint === posEndingPoint) {
return [];
}else if( numberInStartPoint >= 0 && posEndingPoint >= 0) {
posEndingPoint = posEndingPoint;
posStartPoint = numberInStartPoint;
}
for(var i = posStartPoint; i < posEndingPoint; i++){
updateArr.push(array[i]);
}
return updateArr;
}
}
I have these passing test cases:
'if no other arguments , it should return original array':function(){
var arr = [1,2,3];
eq(arr,slice(arr)); // test passed
},
'if second argument is non-integer value , return original array':function(){
var arr = [1,2,3];
eq(arr,slice(arr,'a')); // test passed
},
'if second argument is string but include number only , it should return elements from startingPoint':function(){
var arr = [1,2,3,4];
eq('2,3,4',slice(arr,'1')); // test passed
},
'if startingPoint is integer value , return elements from startingPoint to array.length':function(){
var arr = [1,2,3];
eq('3',slice(arr,2)); // test passed
},
'if startingPoint is negative value, it should count from right side':function(){
var arr = [1,2,3];
eq('2,3',slice(arr,-2)); // test passed
},
'if startingPoint is greater then array.length , return an empty array':function(){
var arr = [1,2,3];
eq('',slice(arr,5)); // test passed
},
'if endingPoint is non-integer value, it should return an empty array':function(){
var arr = [1,2,3];
eq('1,2',slice(arr,0,'2')); // test passed
},
'if endingPoint is negative value , it should start count from right side':function(){
var arr = [1,2,3,4];
eq('1,2',slice(arr,0,-2)); // test passed
},
'if both values are same , return an empty array':function(){
var arr = [1,2,3];
eq('',slice(arr,2,2)); // test passed
},
'if startingPoint and endingPoint both are positive , return element between them exluding endingPoint':function(){
var arr = [1,2,3,4];
eq('',slice(arr,3,0)); // test passed
},
'if startingPoint is negative and endingPoint is positive, it should start counting from right ':function(){
var arr = [1,2,3,4];
eq('2,3,4',slice(arr,-3,4)); // test passed
},
'if both are negative, both point need to start counting from right side ':function(){
var arr = [1,2,3,4];
eq('2,3', slice(arr, -3,-1)); // test passed
},
'if endingPoint is less then startingPoint, it should return empty array.':function(){
var arr = [1,2,3,4];
eq('', slice(arr,3,1)); // test passed
},
'if both are non-integer, it should count from right side if numbers inside string otherwise return empty arr.':function(){
var arr = [1,2,3,4];
eq('2,3', slice(arr,'1','3')); // test passed
eq('',slice(arr,'a','2')); // test passed
}
Have I made a correct implementation of Array.prototype.slice() method? Any test cases that I’ve missed?
I am using tinytest.js library for testing.
Solution
General points
ifstatements that return should not be followed by anelse. Egif (foo) { return bar } else if (bar) {..Theelseis redundant.typeofis not a function it is an operator (language token). Thustypeof(foo) === "foo"is the same astypeof foo === "foo"- Use the function
isNaNto test if a value (number like) can not be parsed to aNumber. - Use constants
constto define values that do not change. Eg you havevar updateArr = [];could beconst updateArr = []; - You should always keep
vardeclarations in one place at the top of the function. If you are defining block scoped variables in the code uselet -
Do not use the
argumentsobject it has some odd quirks that can catch the unwary. Use ES6+ rest parameters, or any of the new ways of defining arguments. example using default parametersfunction slice(array, start = 0, end = array.length) -
Naming is poor and often too verbose. Keep names simple and short. Use the context of the code to add semantic meaning.
To pick an example the arguments
startingPoint,endingPointdoespointadd anything of value to the argument name?. Keep it shortstartas a noun is better than the gerund of startstartingthat implies start as a verb (General naming rule Verbs for functions, nouns for variables).Both arguments would best be
startandendwithin the context of the function their meaning is very clear. -
You are duplicating content. eg You explicitly define 3 return arrays using two names
result,updateArr. You should never do this.
The code is very old school. Keep your skills up to date and always write using the latest language version.
You are repeating logic. Vetting the arguments, iterating the array (there are 3 for loops in the code!!)
Compliance
Your function failed many compliance tests that I performed.
I am not a big fan of unit testing as it is just as subject to error, omission, flawed assumptions as the code it is testing.
In this case despite the effort you put in to test the code, the testing is of no use as the assumptions you have made are incorrect, and the tests are incomplete.
I am surprised that you did not compare the result of your function to the result of Array.slice on the same array. That would be the better than testing against what you assume to be correct, would it not?
Compliance failure list
-
Return same array in many cases. Eg
a = [1,2,3]; if (splice(a) === a) { /* failed */ }
The returned array must never be the same array. Many people usesliceas a replacement to the old[].concat(array)hack. -
Returns incorrect length array. Eg
a = []; if (splice(a, 1, 2).length !== a.length) { /* failed */ }This is due to several incorrect behaviors.
- Fails to detect array like objects, or non arrays.
- Crashes page due to not bounds checking arguments. Eg
slice([],0,Infinity)starts an infinite loop that eventually crashes page due to memory error.
There are other fail modes but they are force edge cases that will have varying results across browsers and versions.
Example
The example implements slice as best can be without actual being a property of the Array being sliced.
This forces the code to have to handle non array or array like arguments. It does its best to imitate slice if the array argument contains that method. Will throw a RangeError if first argument is not an array or a ReferenceError is first argument is missing.
It uses instanceof to vet the first argument.
To vet the inputs it uses some helper functions at the top of the code. vetPos and toPos to convert relative position values to absolute positions as set out by the spec.
function slice(array, start, end) {
if (array instanceof Array) {
const len = array.length;
const toPos = v => Math.max(0, Math.min(len, v < 0 ? len + v : v));
const vetPos = v => toPos(isNaN(v) ? 0 : Math.floor(Math.abs(v)) * Math.sign(v));
start = vetPos(start);
end = vetPos(end === undefined ? len : end);
let count = Math.max(0, end - start), idx = start, res;
if (count === len) { res = [...array] }
else {
res = [];
while (count--) { res.push(array[idx++]) }
}
return res;
}
if (array && typeof array.slice === "function") { return array.slice(start, end) }
if (array) { throw new RangeError("First argument must be an array") }
throw new ReferenceError("Missing first argument");
}
Notes
-
Reference for compliance from MDN
Array.sliceand the latest spec ECMAScript 2020 (Draft) ‘Array.prototype.slice’ -
Because your code was messed up due to tabs when you pasted to CR (Good reason to use spaces rather than tabs) I auto formatted the code so I could read it and thus may have missed some related style points.