Array Stutter ImplementationPrinting a 2D arrayTesting if numbers in the array can be added up to equal the largest number in the arrayAnagram counterAccess an associative array value given an array of keys in PHPJavascript node/react web developer interview codeFilter array elements using variable number of filtersBit array implementationCount duplicates in a JavaScript arrayModify array of arrays to create custom array of objectsTwo-sum solution in JavaScript

Multi tool use
Multi tool use

Pros and cons of writing a book review?

Working in the USA for living expenses only; allowed on VWP?

X-shaped crossword

Is it legal in the UK for politicians to lie to the public for political gain?

Bent spoke design wheels — feasible?

Company is asking me to work from overseas, but wants me to take a paycut

Riley's, assemble!

How to decline physical affection from a child whose parents are pressuring them?

If Boris Johnson were prosecuted and convicted of lying about Brexit, can that be used to cancel Brexit?

Pronoun introduced before its antecedent

How photons get into the eyes?

Is it a problem that pull requests are approved without any comments

How do you build a story from a world?

What's the logic behind the the organization of Hamburg's bus transport into "rings"?

In this example, which path would a monster affected by the Dissonant Whispers spell take?

How to make a setting relevant?

Convert camelCase and PascalCase to Title Case

How to skip replacing first occurrence of a character in each line?

What happened to all the nuclear material being smuggled after the fall of the USSR?

PhD student with mental health issues and bad performance

Building a road to escape Earth's gravity by making a pyramid on Antartica

Can't access wrapper list in test method

Is there any word or phrase for negative bearing?

How to make thick Asian sauces?



Array Stutter Implementation


Printing a 2D arrayTesting if numbers in the array can be added up to equal the largest number in the arrayAnagram counterAccess an associative array value given an array of keys in PHPJavascript node/react web developer interview codeFilter array elements using variable number of filtersBit array implementationCount duplicates in a JavaScript arrayModify array of arrays to create custom array of objectsTwo-sum solution in JavaScript






.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;








5












$begingroup$


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!










share|improve this question









$endgroup$











  • $begingroup$
    Your comment about the last two elements being the same triggered me to test an input with the final elements repeating and I found a bug: stutter(["buffalo", "buffalo", "buffalo", "buffalo"]) returns an array with 7 elements.
    $endgroup$
    – CompuChip
    May 27 at 7:49

















5












$begingroup$


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!










share|improve this question









$endgroup$











  • $begingroup$
    Your comment about the last two elements being the same triggered me to test an input with the final elements repeating and I found a bug: stutter(["buffalo", "buffalo", "buffalo", "buffalo"]) returns an array with 7 elements.
    $endgroup$
    – CompuChip
    May 27 at 7:49













5












5








5





$begingroup$


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!










share|improve this question









$endgroup$




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!







javascript array homework






share|improve this question













share|improve this question











share|improve this question




share|improve this question










asked May 26 at 14:46









David WhiteDavid White

819622




819622











  • $begingroup$
    Your comment about the last two elements being the same triggered me to test an input with the final elements repeating and I found a bug: stutter(["buffalo", "buffalo", "buffalo", "buffalo"]) returns an array with 7 elements.
    $endgroup$
    – CompuChip
    May 27 at 7:49
















  • $begingroup$
    Your comment about the last two elements being the same triggered me to test an input with the final elements repeating and I found a bug: stutter(["buffalo", "buffalo", "buffalo", "buffalo"]) returns an array with 7 elements.
    $endgroup$
    – CompuChip
    May 27 at 7:49















$begingroup$
Your comment about the last two elements being the same triggered me to test an input with the final elements repeating and I found a bug: stutter(["buffalo", "buffalo", "buffalo", "buffalo"]) returns an array with 7 elements.
$endgroup$
– CompuChip
May 27 at 7:49




$begingroup$
Your comment about the last two elements being the same triggered me to test an input with the final elements repeating and I found a bug: stutter(["buffalo", "buffalo", "buffalo", "buffalo"]) returns an array with 7 elements.
$endgroup$
– CompuChip
May 27 at 7:49










2 Answers
2






active

oldest

votes


















11












$begingroup$

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.






share|improve this answer










New contributor



Pritilender is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.





$endgroup$








  • 1




    $begingroup$
    Great answer and welcome to CR! There's also [].concat(...arr.map(e => [e, e])).
    $endgroup$
    – ggorlen
    May 26 at 22:27






  • 2




    $begingroup$
    "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." Well this is in the requirments, so wether you like it or not, that's what is expected. If you read the task it does not tell you to create a new array it says, that the array that is given to the function should now contain different data.
    $endgroup$
    – Andrew Savinykh
    May 27 at 5:04






  • 2




    $begingroup$
    @IsmaelMiguel for ... of is different than for ... in - for ... of loops using the Symbol.Iterator - it's meant to loop through things like Arrays, Maps, and Sets, without running into the issues of for ... in
    $endgroup$
    – Daniel
    May 27 at 5:53






  • 1




    $begingroup$
    @Daniel You are right, I misread it completely. I've removed my (embarassingly) innacurate comment. :x
    $endgroup$
    – Ismael Miguel
    May 27 at 8:31






  • 1




    $begingroup$
    Also, your example of mutating the original array doesn't actually mutate it. You'd need to use splice or somesuch to insert into the original. (I'd just choose not to interpret the assignment that way; it's really not good coding)
    $endgroup$
    – JollyJoker
    May 27 at 9:50


















4












$begingroup$

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;






share|improve this answer









$endgroup$













    Your Answer






    StackExchange.ifUsing("editor", function ()
    StackExchange.using("externalEditor", function ()
    StackExchange.using("snippets", function ()
    StackExchange.snippets.init();
    );
    );
    , "code-snippets");

    StackExchange.ready(function()
    var channelOptions =
    tags: "".split(" "),
    id: "196"
    ;
    initTagRenderer("".split(" "), "".split(" "), channelOptions);

    StackExchange.using("externalEditor", function()
    // Have to fire editor after snippets, if snippets enabled
    if (StackExchange.settings.snippets.snippetsEnabled)
    StackExchange.using("snippets", function()
    createEditor();
    );

    else
    createEditor();

    );

    function createEditor()
    StackExchange.prepareEditor(
    heartbeatType: 'answer',
    autoActivateHeartbeat: false,
    convertImagesToLinks: false,
    noModals: true,
    showLowRepImageUploadWarning: true,
    reputationToPostImages: null,
    bindNavPrevention: true,
    postfix: "",
    imageUploader:
    brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
    contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
    allowUrls: true
    ,
    onDemand: true,
    discardSelector: ".discard-answer"
    ,immediatelyShowMarkdownHelp:true
    );



    );













    draft saved

    draft discarded


















    StackExchange.ready(
    function ()
    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f221064%2farray-stutter-implementation%23new-answer', 'question_page');

    );

    Post as a guest















    Required, but never shown

























    2 Answers
    2






    active

    oldest

    votes








    2 Answers
    2






    active

    oldest

    votes









    active

    oldest

    votes






    active

    oldest

    votes









    11












    $begingroup$

    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.






    share|improve this answer










    New contributor



    Pritilender is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
    Check out our Code of Conduct.





    $endgroup$








    • 1




      $begingroup$
      Great answer and welcome to CR! There's also [].concat(...arr.map(e => [e, e])).
      $endgroup$
      – ggorlen
      May 26 at 22:27






    • 2




      $begingroup$
      "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." Well this is in the requirments, so wether you like it or not, that's what is expected. If you read the task it does not tell you to create a new array it says, that the array that is given to the function should now contain different data.
      $endgroup$
      – Andrew Savinykh
      May 27 at 5:04






    • 2




      $begingroup$
      @IsmaelMiguel for ... of is different than for ... in - for ... of loops using the Symbol.Iterator - it's meant to loop through things like Arrays, Maps, and Sets, without running into the issues of for ... in
      $endgroup$
      – Daniel
      May 27 at 5:53






    • 1




      $begingroup$
      @Daniel You are right, I misread it completely. I've removed my (embarassingly) innacurate comment. :x
      $endgroup$
      – Ismael Miguel
      May 27 at 8:31






    • 1




      $begingroup$
      Also, your example of mutating the original array doesn't actually mutate it. You'd need to use splice or somesuch to insert into the original. (I'd just choose not to interpret the assignment that way; it's really not good coding)
      $endgroup$
      – JollyJoker
      May 27 at 9:50















    11












    $begingroup$

    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.






    share|improve this answer










    New contributor



    Pritilender is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
    Check out our Code of Conduct.





    $endgroup$








    • 1




      $begingroup$
      Great answer and welcome to CR! There's also [].concat(...arr.map(e => [e, e])).
      $endgroup$
      – ggorlen
      May 26 at 22:27






    • 2




      $begingroup$
      "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." Well this is in the requirments, so wether you like it or not, that's what is expected. If you read the task it does not tell you to create a new array it says, that the array that is given to the function should now contain different data.
      $endgroup$
      – Andrew Savinykh
      May 27 at 5:04






    • 2




      $begingroup$
      @IsmaelMiguel for ... of is different than for ... in - for ... of loops using the Symbol.Iterator - it's meant to loop through things like Arrays, Maps, and Sets, without running into the issues of for ... in
      $endgroup$
      – Daniel
      May 27 at 5:53






    • 1




      $begingroup$
      @Daniel You are right, I misread it completely. I've removed my (embarassingly) innacurate comment. :x
      $endgroup$
      – Ismael Miguel
      May 27 at 8:31






    • 1




      $begingroup$
      Also, your example of mutating the original array doesn't actually mutate it. You'd need to use splice or somesuch to insert into the original. (I'd just choose not to interpret the assignment that way; it's really not good coding)
      $endgroup$
      – JollyJoker
      May 27 at 9:50













    11












    11








    11





    $begingroup$

    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.






    share|improve this answer










    New contributor



    Pritilender is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
    Check out our Code of Conduct.





    $endgroup$



    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.







    share|improve this answer










    New contributor



    Pritilender is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
    Check out our Code of Conduct.








    share|improve this answer



    share|improve this answer








    edited May 27 at 10:16





















    New contributor



    Pritilender is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
    Check out our Code of Conduct.








    answered May 26 at 16:36









    PritilenderPritilender

    18616




    18616




    New contributor



    Pritilender is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
    Check out our Code of Conduct.




    New contributor




    Pritilender is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
    Check out our Code of Conduct.









    • 1




      $begingroup$
      Great answer and welcome to CR! There's also [].concat(...arr.map(e => [e, e])).
      $endgroup$
      – ggorlen
      May 26 at 22:27






    • 2




      $begingroup$
      "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." Well this is in the requirments, so wether you like it or not, that's what is expected. If you read the task it does not tell you to create a new array it says, that the array that is given to the function should now contain different data.
      $endgroup$
      – Andrew Savinykh
      May 27 at 5:04






    • 2




      $begingroup$
      @IsmaelMiguel for ... of is different than for ... in - for ... of loops using the Symbol.Iterator - it's meant to loop through things like Arrays, Maps, and Sets, without running into the issues of for ... in
      $endgroup$
      – Daniel
      May 27 at 5:53






    • 1




      $begingroup$
      @Daniel You are right, I misread it completely. I've removed my (embarassingly) innacurate comment. :x
      $endgroup$
      – Ismael Miguel
      May 27 at 8:31






    • 1




      $begingroup$
      Also, your example of mutating the original array doesn't actually mutate it. You'd need to use splice or somesuch to insert into the original. (I'd just choose not to interpret the assignment that way; it's really not good coding)
      $endgroup$
      – JollyJoker
      May 27 at 9:50












    • 1




      $begingroup$
      Great answer and welcome to CR! There's also [].concat(...arr.map(e => [e, e])).
      $endgroup$
      – ggorlen
      May 26 at 22:27






    • 2




      $begingroup$
      "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." Well this is in the requirments, so wether you like it or not, that's what is expected. If you read the task it does not tell you to create a new array it says, that the array that is given to the function should now contain different data.
      $endgroup$
      – Andrew Savinykh
      May 27 at 5:04






    • 2




      $begingroup$
      @IsmaelMiguel for ... of is different than for ... in - for ... of loops using the Symbol.Iterator - it's meant to loop through things like Arrays, Maps, and Sets, without running into the issues of for ... in
      $endgroup$
      – Daniel
      May 27 at 5:53






    • 1




      $begingroup$
      @Daniel You are right, I misread it completely. I've removed my (embarassingly) innacurate comment. :x
      $endgroup$
      – Ismael Miguel
      May 27 at 8:31






    • 1




      $begingroup$
      Also, your example of mutating the original array doesn't actually mutate it. You'd need to use splice or somesuch to insert into the original. (I'd just choose not to interpret the assignment that way; it's really not good coding)
      $endgroup$
      – JollyJoker
      May 27 at 9:50







    1




    1




    $begingroup$
    Great answer and welcome to CR! There's also [].concat(...arr.map(e => [e, e])).
    $endgroup$
    – ggorlen
    May 26 at 22:27




    $begingroup$
    Great answer and welcome to CR! There's also [].concat(...arr.map(e => [e, e])).
    $endgroup$
    – ggorlen
    May 26 at 22:27




    2




    2




    $begingroup$
    "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." Well this is in the requirments, so wether you like it or not, that's what is expected. If you read the task it does not tell you to create a new array it says, that the array that is given to the function should now contain different data.
    $endgroup$
    – Andrew Savinykh
    May 27 at 5:04




    $begingroup$
    "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." Well this is in the requirments, so wether you like it or not, that's what is expected. If you read the task it does not tell you to create a new array it says, that the array that is given to the function should now contain different data.
    $endgroup$
    – Andrew Savinykh
    May 27 at 5:04




    2




    2




    $begingroup$
    @IsmaelMiguel for ... of is different than for ... in - for ... of loops using the Symbol.Iterator - it's meant to loop through things like Arrays, Maps, and Sets, without running into the issues of for ... in
    $endgroup$
    – Daniel
    May 27 at 5:53




    $begingroup$
    @IsmaelMiguel for ... of is different than for ... in - for ... of loops using the Symbol.Iterator - it's meant to loop through things like Arrays, Maps, and Sets, without running into the issues of for ... in
    $endgroup$
    – Daniel
    May 27 at 5:53




    1




    1




    $begingroup$
    @Daniel You are right, I misread it completely. I've removed my (embarassingly) innacurate comment. :x
    $endgroup$
    – Ismael Miguel
    May 27 at 8:31




    $begingroup$
    @Daniel You are right, I misread it completely. I've removed my (embarassingly) innacurate comment. :x
    $endgroup$
    – Ismael Miguel
    May 27 at 8:31




    1




    1




    $begingroup$
    Also, your example of mutating the original array doesn't actually mutate it. You'd need to use splice or somesuch to insert into the original. (I'd just choose not to interpret the assignment that way; it's really not good coding)
    $endgroup$
    – JollyJoker
    May 27 at 9:50




    $begingroup$
    Also, your example of mutating the original array doesn't actually mutate it. You'd need to use splice or somesuch to insert into the original. (I'd just choose not to interpret the assignment that way; it's really not good coding)
    $endgroup$
    – JollyJoker
    May 27 at 9:50













    4












    $begingroup$

    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;






    share|improve this answer









    $endgroup$

















      4












      $begingroup$

      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;






      share|improve this answer









      $endgroup$















        4












        4








        4





        $begingroup$

        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;






        share|improve this answer









        $endgroup$



        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;







        share|improve this answer












        share|improve this answer



        share|improve this answer










        answered May 27 at 12:07









        Blindman67Blindman67

        11.4k1623




        11.4k1623



























            draft saved

            draft discarded
















































            Thanks for contributing an answer to Code Review Stack Exchange!


            • Please be sure to answer the question. Provide details and share your research!

            But avoid


            • Asking for help, clarification, or responding to other answers.

            • Making statements based on opinion; back them up with references or personal experience.

            Use MathJax to format equations. MathJax reference.


            To learn more, see our tips on writing great answers.




            draft saved


            draft discarded














            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f221064%2farray-stutter-implementation%23new-answer', 'question_page');

            );

            Post as a guest















            Required, but never shown





















































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown

































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown







            o7HB5Cl1p1wt IK oU qL454
            zc7UbYQhpyukfNw

            Popular posts from this blog

            Category:9 (number) SubcategoriesMedia in category "9 (number)"Navigation menuUpload mediaGND ID: 4485639-8Library of Congress authority ID: sh85091979ReasonatorScholiaStatistics

            Circuit construction for execution of conditional statements using least significant bitHow are two different registers being used as “control”?How exactly is the stated composite state of the two registers being produced using the $R_zz$ controlled rotations?Efficiently performing controlled rotations in HHLWould this quantum algorithm implementation work?How to prepare a superposed states of odd integers from $1$ to $sqrtN$?Why is this implementation of the order finding algorithm not working?Circuit construction for Hamiltonian simulationHow can I invert the least significant bit of a certain term of a superposed state?Implementing an oracleImplementing a controlled sum operation

            Magento 2 “No Payment Methods” in Admin New OrderHow to integrate Paypal Express Checkout with the Magento APIMagento 1.5 - Sales > Order > edit order and shipping methods disappearAuto Invoice Check/Money Order Payment methodAdd more simple payment methods?Shipping methods not showingWhat should I do to change payment methods if changing the configuration has no effects?1.9 - No Payment Methods showing upMy Payment Methods not Showing for downloadable/virtual product when checkout?Magento2 API to access internal payment methodHow to call an existing payment methods in the registration form?