Ttdfjt

I was assigned this homework assignment to complete. The question originates from CodeStepByStep. Below is the prompt for the question:

Write a function stutter that takes an array of Strings as a parameter
and that replaces every String with two of that String. For example,
if an array stores the values ["how", "are", "you?"] before the
function is called, it should store the values ["how", "how", "are",
"are", "you?", "you?"] after the function finishes executing.

Below is my implementation:

function stutter(arr) 
 if(arr.length == 0) return []; 
 if(arr.length == 1) 
 arr.push(arr[0]);
 return arr;
 
 let size = arr.length;
 for(let i = 0; i < size + 2; i += 2) 
 arr.splice(i + 1, 0, arr[i]);
 
 //If last two elements are not the same
 if(arr[arr.length - 2] != arr[arr.length - 1] && arr.length != 1) 
 arr.push(arr[arr.length - 1]); 
 
 return arr;

It would be really helpful if I could get some feedback on how my code is written. I am fairly new to JavaScript, and I don't know some of the functions that could have made this a lot easier. Feedback on code efficiency and the implementation itself is warmly invited!

By reviewing your code, the first thing that comes to my mind is a lot of if statements. You should try and keep those minimal by writing solutions to be as general as they can be.

Another thing that I don't like in your code is that you are doing manipulation of array passed to the function, instead of going out with a fresh one. It looks like this manipulation is what leads to a lot of ifs in the first place.

So main point on how you can refactor your code is that you initialize an empty array which will act as a result and then manipulate that code:

function stutter(arr) 
 const result = []

 for (let word of arr) // for..of loop is a bit clearer to read
 result.push(word, word) // push can accept N arguments
 

 return result

So by initalizing resulting array with an empty one, you cleared of a case that the argument array passed in is empty, because for..of loop won't do the looping at all.
I've used for..of loop here since it's less code, but you could also use the C-like for like you've written in your question. Note that that loop also wouldn't loop if the argument array was empty, therefor no need for the if(arr.length == 0).

By the way, I'm a bit puzzled with what exactly is the point of the last if you have in the code, but I think that with the refactoring I've provided, then there is no need for it at all.

Since you've asked for more JS way of doing this, here are two ways:

Using map and flat (this one won't run in Edge):

function stutter(arr) 
 return arr.map(x => [x, x]).flat()

Using just reduce:

function stutter(arr) 
 return arr.reduce((result, current) => [...result, current, current], [])

EDIT

As @AndrewSavinykh pointed out, the assignment is worded in such way that the original array should be mutated. If that truly is the case, and both Andrew and me are not misreading it, then the solution would be to just reassign the result to arr and return arr (@JollyJoker pointed that previous answer was not correct) use splice (the code is updated for the first example, but same thing can be done for the other two):

function stutter(arr) 
 const result = []

 for (let word of arr) // for..of loop is a bit clearer to read
 result.push(word, word) // push can accept N arguments
 

 arr.splice(0, arr.length, result)
 return arr

On the other hand, I'd advise against input arguments mutation, because you can easily forget which function mutates the arguments and which not, so it can create some additional cognitive load while reading the code. Also, it goes against the principles of functional programming where you want your functions to be as pure as they can.

General Points

• Use strict equality and inequality, === or !== rather than == or !=




• Use const for variables that do not change. Eg let size = can be const size =




• Spaces between for( and if( eg for ( and if (

Problem/Bug

Your code has a very serious and hard to spot bug.

It seams to me that the array should be modified in place (which is the point of the exercise) which for the most part you do. However when the array size is zero you return a new array. I would call it a bug as it could have serious consequences in any code that manipulates or relies on the content of the referenced array.

The line should have been

if (arr.length == 0) return arr 

Complexity

Array.splice is an expensive operation as it needs to move each item above the splice point. If you splice each item in the array you end up with high complexity.

This is compounded with how JS arrays grow. When the length is changed the JS engine checks if there is enough pre allocated space, if there is not, it doubles the allocation space, moves the old array to the new memory (if needed). That means that a single push can (if at the pre allocated boundary) cause the entire array to be iterated.

From the top

The problem is that modifying the array in place means that you need to avoid losing the original content while you copy. This can be done by splicing (as you have done) which has a high time complexity, or by creating a copy of either the original array.

If you start from the top of the array the copied items will always be at an index above the original position so you will not need to deal with the problem of overwriting the content as you copy.

Examples

The following stutters the array from top to bottom, growing the array via its length property. However some JS engines my create a sparse array so the second example grows the array by pushing itself onto the top

function stutter(arr) 
 var iFrom = arr.length, iTo = iFrom * 2 - 1;
 arr.length *= 2;
 while (iFrom-- > 0) arr[iTo--] = arr[iTo--] = arr[iFrom] 
 return arr;

Using push to grow

function stutter(arr) 
 var iFrom = arr.length, iTo = iFrom * 2 - 1;
 arr.push(...arr);
 while (iFrom-- > 0) arr[iTo--] = arr[iTo--] = arr[iFrom] 
 return arr;

What you should not do.

This example does not grow the array before adding to it. From a JS point of view the result is identical. However the array will be mutated into a sparse array which for large arrays can represent a serious performance reduction with any code that needs to access the array.

function stutter(arr) 
 var iFrom = arr.length, iTo = iFrom * 2 - 1;
 while (iFrom-- > 0) arr[iTo--] = arr[iTo--] = arr[iFrom] 
 return arr;