Replace array elements with onclick

Question

I am trying to display new list of shuffled array on onclick but my code is just appending the new shuffled array below the previous list. My code is:

function shuffleArray() {
        var array = ['1','2','3','4'];
            for (var i = array.length - 1; i > 0; i--) {
                var j = Math.floor(Math.random() * (i + 1));
                var temp = array[i];
                array[i] = array[j];
                array[j] = temp;
                }


    for (i=0;i<array.length;i++) {
            var span = document.createElement('span');
            span.innerHTML = array[i];
            span.onclick = shuffleArray; //calls the same function
            var div = document.createElement('div');
            div.appendChild(span);
            document.body.appendChild(div);
            }
    }

Current output:

    2
    1
    4 // if I click here then results gets appended below
    3
    3
    2
    4
    1 // if I click here then results gets appended below
    2
    1
    4
    3 // and so on

Desired output: I want the page content to get updated with new array elements, each time I click some array element.

I had probably figured that out long before but if I don't use append function, then onclick doesn't works. — user2567857
– user2567857, Commented May 12, 2014 at 7:57
Place your new elements in an outer wrapping div, and then prior to appending new children call wrapping_div.innerHTML='';. Either that, or store each div you append to an array and then, when you need to remove them, step through the array using body.removeChild(div);. — Pebbl
– Pebbl, Commented May 12, 2014 at 8:02

Pebbl · Accepted Answer · 2014-05-12 08:25:13Z

1

What follows is the simple version, but this is prone to memory leaks because you are using .onclick and are not removing those references before destroying the elements with .innerHTML='';

var wrapping_div = document.createElement('div');

document.body.appendChild(wrapping_div);

function shuffleArray() {
  var array = ['1','2','3','4'];
  for (var i = array.length - 1; i > 0; i--) {
    var j = Math.floor(Math.random() * (i + 1));
    var temp = array[i];
    array[i] = array[j];
    array[j] = temp;
  }
  wrapping_div.innerHTML = '';
  for (i=0;i<array.length;i++) {
    var span = document.createElement('span');
    span.innerHTML = array[i];
    span.onclick = shuffleArray; //calls the same function
    var div = document.createElement('div');
    div.appendChild(span);
    wrapping_div.appendChild(div);
  }
}

shuffleArray();

A better way is http://jsfiddle.net/NCUDv/2/:

var wrapping_div = document.createElement('div');

document.body.appendChild(wrapping_div);

function shuffleArray() {
  var i, j, temp, span, div,
      c = wrapping_div.childNodes, 
      l = c.length,
      array = ['1','2','3','4'], 
      k = array.length;
  for (i=k-1; i>=0; i--) {
    j = Math.floor(Math.random() * (i + 1));
    temp = array[i];
    array[i] = array[j];
    array[j] = temp;
  }
  for (i=l-1; i>=0; i-- ) {
    c[i].firstChild.removeEventListener('click', shuffleArray);
    wrapping_div.removeChild(c[i]);
  }
  for (i=0; i<k; i++) {
    span = document.createElement('span');
    span.innerHTML = array[i];
    span.addEventListener('click', shuffleArray);
    div = document.createElement('div');
    div.appendChild(span);
    wrapping_div.appendChild(div);
  }
}

shuffleArray();

edited May 12, 2014 at 8:25

answered May 12, 2014 at 8:13

Pebbl

36.1k6 gold badges66 silver badges65 bronze badges

Sign up to request clarification or add additional context in comments.

9 Comments

user3299259 Over a year ago

sorry, i didnt see you post the same answer 1 min before ;).... your second way would be pure dom scripting, but is definitley the slower one

user2567857 Over a year ago

@ThorstenArtner'Austria' is your approach more efficient than the second method?

user3299259 Over a year ago

its the same pebbl posted in his first way, perhaps he could give a link about the memory leaks generated by not removeing the eventListeners, i never noticed this. its stuff for the JS garbage collector

Pebbl Over a year ago

@ThorstenArtner'Austria' ~ Ah np :) Using .innerHTML will be faster (precisely because it doesn't wipe things safely), I have used it myself in the past for small areas to be blanked (mainly for elements without event listeners). However, in any code that is going to repeat itself over and over and that uses event listeners (like this), memory problems should be avoided at all costs. What may be a best of both worlds would be to not keep creating and destroying the elements, just change their innerHTML value directly i.e. 1 becomes 4, 2 becomes 3 and so on.

user3299259 Over a year ago

@pebbl did you ever have problems by not removing the eventListeners, or could you please give a link to an science article about the memory leaks

|

score 1 · Accepted Answer · 2014-05-12 08:46:24Z

1

var container=document.createElement ("div");
document.body.appendChild (container);

function shuffleArray() {
    var array = ['1','2','3','4'], l=array.length;
    var i,j,tmp;
    for (i = l - 1; i > 0; i--) {
        j = Math.floor(Math.random() * (i + 1));
        temp = array[i];
        array[i] = array[j];
        array[j] = temp;
        }

    var span;
    container.innerHTML="";
    for (i=0;i<l;i++) {
        span = document.createElement('span');
        span.innerHTML = array[i];
        span.onclick = shuffleArray; //calls the same function
        container.appendChild(span);
        }
    }

edited

var container=document.createElement ("div");
document.body.appendChild (container);

function shuffleArray() {
    var array = ['1','2','3','4'], l=array.length;
    var i,j,tmp;
    for (i = l - 1; i > 0; i--) {
        j = Math.floor(Math.random() * (i + 1));
        temp = array[i];
        array[i] = array[j];
        array[j] = temp;
        }

    var p;
    container.innerHTML="";
    for (i=0;i<l;i++) {
        p = document.createElement('p');
        p.style.margin="0px";
        p.innerHTML = array[i];
        p.onclick = shuffleArray; //calls the same function
        container.appendChild(span);
        }
    }

edited May 12, 2014 at 8:46

answered May 12, 2014 at 8:15

user3299259

4 Comments

user2567857 Over a year ago

Here, the elements are displayed horizontally. I wanted it to be displayed horizontally.

user3299259 Over a year ago

Do you mean vertically then change the span to p

user2567857 Over a year ago

Sorry, vertically and probably with no spaces between 2 vertical elements! Incase of p, it creates vertical spaces.

user3299259 Over a year ago

this is stuff of CSS. margin Property

Collectives™ on Stack Overflow

Replace array elements with onclick

2 Answers 2

9 Comments

4 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

9 Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related