for loop causing multiple creation of object

Question

I have an array containing messages that I print to the screen randomly one at a time. I am using javascript to create a new DOM element ('p') and then to create a new text node that I attach to the new p element.

In order to not display a duplicate, I created a new array that will hold the new text that is displayed. Prior to printing to the screen, I check to make sure that the text that is about to be displayed is not currently in the new array so that I can check for duplicates.

It is never finding a duplicate however because each time it goes through the for loop it is creating another object (I believe it is a text node because each iteration through it will console log me the text nodes content +=1 each time. For instance, the first time might get:

"this is the message"

and the next console log I will get:

"this is the message" "this is the message"

and the next time:

"this is the message" "this is the message" "this is the message"

Here is the code I am using for that portion as I believe this is the only relevant code:

function randomMessages(){
    var displayMessage = document.getElementById('front-message');
    var newParagraph = document.createElement('p');
    newParagraph.setAttribute('class', 'front-page-messages');
    currentMessage = Math.floor(Math.random() * frontPageMessages.length);
    var randomText = document.createTextNode(frontPageMessages[currentMessage]);
    checkPastMessages(oldMessages, randomText, newParagraph); // check new array containing displayed messages for duplicates
    newParagraph.appendChild(randomText);
    displayMessage.appendChild(newParagraph);
    newParagraph.style.position = "relative";
    newParagraph.style.top = Math.floor(Math.random() * (window.innerHeight -430) ) + "px";
    newParagraph.style.textAlign = "center";
    newParagraph.style.fontFamily ="'Montserrat', sans-serif";
    newParagraph.style.fontSize = "1.3em";
    newParagraph.style.color = "#52A8EC";
    oldMessages.push(randomText);
    console.log(oldMessages);
    window.setTimeout(function(){
        displayMessage.removeChild(newParagraph);
        newParagraph.removeChild(randomText);
        randomMessages();
    }, 5000);
}
randomMessages();

function checkPastMessages(oldMessages, randomText, newParagraph){
    for(var i = 0; i < oldMessages.length; i++){
        if(randomText == oldMessages[i]){
            console.log('duplicate');
            randomMessages();
        }else{
            console.log('not a duplicate');
            console.log(randomText);
        };

    };

}

Whenever your checkPastMessages() script finds a duplicate, it starts up a new randomMessages() cycle. I don't understand why. — Pointy
– Pointy, Commented Sep 19, 2015 at 16:01
The intention was to rerun randomMessages() if the array already contained the text, but I see in doing so I am creating more objects because I have not removed them. I still need to figure out how to do that without deleting the element before it is used in the event it is not a duplicate. (edit - I just added the remove elements to the in the event a duplicate is found but my original problem still exists.) — user5096599
– user5096599, Commented Sep 19, 2015 at 16:02
Text nodes are objects, which are compared by reference. You should instead compare their nodeValues, or store the used indices (currentMessage). — Oriol
– Oriol, Commented Sep 19, 2015 at 16:03
You could also order your messages randomly up front, and then just 'pop' one off the list every 5 seconds. — Kenney
– Kenney, Commented Sep 19, 2015 at 16:04
Btw, what should happen when all messages from array are displayed, reset all, and start new cycle? — sinisake
– sinisake, Commented Sep 19, 2015 at 16:07

Leo Nix · Accepted Answer · 2015-09-19 19:28:52Z

1

change line: checkPastMessages(oldMessages, randomText, newParagraph)
to:  checkPastMessages(oldMessages, randomText.textContent, newParagraph)

and

change line: oldMessages.push(randomText)
to: oldMessages.push(randomText.textContent)

this way the following line would be comparing string literals as opposed to objects:

if(randomText == oldMessages[i]) {

you won't get equality between two objects comparison. Try the following examples in a console:

var o=document.createTextNode("abc");
var b=document.createTextNode("abc");

console.log(o==b); //should yield false;
console.log(o.textContent==b.textContent); //should yield true

var c = o;
console.log(c == o); //should yield true since they both point to the same object address (reference).

edited Sep 19, 2015 at 19:28

answered Sep 19, 2015 at 16:18

Leo Nix

2,1053 gold badges24 silver badges37 bronze badges

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

Comments

Gogowitsch · Accepted Answer · 2015-09-19 16:10:49Z

0

The solution is to add the string frontPageMessages[currentMessage] or the index currentMessage to the list oldMessages.

It currently doesn't work because the object comparison in checkPastMessages doesn't see the identical nodes as identical. What adds to the confusion is that console.log treats text nodes as if they were strings.

answered Sep 19, 2015 at 16:10

Gogowitsch

1,2331 gold badge14 silver badges35 bronze badges

Collectives™ on Stack Overflow

for loop causing multiple creation of object

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related