Javascript for in loop question

Ok, so I want know why “undefined” appears at the beginning of the list that is created using the Javascript for in loop.

Below is the HTML code:

<p id="demo"></p>

Below is the Javascript code:

var names = {
    a: "Angela",
    b: "Billy",
    c: "Charles",
    d: "Derrick"
};

for(var x in names){
	if(typeof names[x] === "string"){
		var text;
		text += names[x] + "<br />";
		document.getElementById("demo").innerHTML = text;
	};
};

This is what it ouputs, exactly as it appears:

undefinedAngela
Billy
Charles
Derrick

Thanks

Maybe because you define the text var without initialising it? And then add to it with +?

Try and see if it makes a difference if you define it like:

var text = "";

converting undefined into a string results in the string "undefined".

but tbh, the given case were better suited to an array than an object. where you could do:

document.getElementById("demo").innerHTML = names.join('<br>');

Shouldn’t the variable ‘text’ be defined outside the loop?

e.g.

var text = "";
for(var x in names){
if(typeof names[x] === "string"){
text += names[x] + "<br />";
document.getElementById("demo").innerHTML = text;
};
};

Just an uneducated guess :smile:

ok, so placing the variable “text” outside of the loop and initializing it worked.

Here is the Javascript code:
var text = “”;

for(var x in names){
if(typeof names === “string”){
text += names + “<br />”;
document.getElementById(“demo”).innerHTML = text;
};
};

And this was for learning purposes only, there isn’t anything in particular that I’m trying to do which is why I didn’t use an array.

Thanks

In that case, you will want to move the innerHTML part out of the loop too, which results in a performance improvement as the document is not being updated every time the code loops through.

Also, the semicolons on the end of braces aren’t valid, and should be removed.

var text = "";

for(var x in names) {
    if(typeof names[x] === "string") {
        text += names[x] + "<br />";
        document.getElementById("demo").innerHTML = text;
    }
}

The typeof part doesn’t seem to be needed either, and can be removed. Though normally it should be replaced by the standard object protection where you check hasOwnProperty on the object to see if the property is directly involved.

And as we’re dealing with properties, we can help to make the code clearer and easier to understand by renaming x to be prop instead.

And while we’re cleaning things up, defining vars inside the for statement tends to be a bit of a no-no, so we’ll define it with the other vars.

var text = "",
    prop = "";

for (prop in names) {
    if (names.hasOwnProperty(prop)) {
        text += names[prop] + "<br />";
    }
}
document.getElementById("demo").innerHTML = text;
1 Like

That would be the right place to declare it using let instead of var but not all browsers support let yet.

An option here is to write your code in ES6 syntax, and use Babel to convert it to ES5-compatible code for the web browsers.

I was checking out the code in jslint, and found that the for…in structure is no longer allowed there. Some investigation revealed that it’s due to them being experimental, with Object.keys() being preferred to use instead.

As a result the code changes to be:

var text = "";

Object.keys(names).forEach(function (key) {
    text += names[key] + "<br />";
});
document.getElementById("demo").innerHTML = text;

Which means that we can now use a map, to convert the array of keys in to values:

document.getElementById("demo").innerHTML = Object.keys(names).map(function (key) {
    return names[key]
}).join('<br>');

But that code while more condensed, is more difficult to understand, and we still have a hard-coded ‘names’ variable within the function.

We can pull out the code to a separate function, so that we can bind the names object to the function and use the this keyword, to have a more generic solution:

function key2values(key) {
    return this[key]
}
document.getElementById("demo").innerHTML = Object.keys(names).map(key2values.bind(names)).join('<br>');

We’re getting close now. We can now pull out the Object.keys part to its own function which gives us direct access to a generic function property for the ‘names’ variable, so we can put back the key2values part and remove the bind section, and we’re good to go.

function objValues(obj) {
    return Object.keys(obj).map(function (key) {
        return obj[key];
    });
}
document.getElementById("demo").innerHTML = objValues(names).join('<br>');

We now have a separate function that’s dedicated to doing just one thing, and doing it well - to retrieve the values of an object as an array. Try it out at http://jsfiddle.net/0tde64hp/

It is tempting to add it as a method of Object so that like Object.keys, you can use it as Object.values. The only problem here though is that extending native objects is a big no-no for compatibility reasons, so keeping it as a separate function is at this stage the right thing to do.

That’s a nice improvement to the code now, and is even a function that we can consider adding to our own set of useful snippets, thanks to inspiration from an exploration using jslint.

3 Likes

put var text=“”; instead of var text; and run code

Where does that appear in the code?

function objValues(obj) {
    return Object.keys(obj).map(function (key) {
        return obj[key];
    });
}
document.getElementById("demo").innerHTML = objValues(names).join('<br>');

There is no reference to text there.

I believe that he was meaning in reference to post #1. If you don’t start with an empty string then things are going to go awry.

But that part was resolved in post #2. The code has moved on a long way since then.

You’re dead right there. For some people though the scrollbar seems to be a feature of web browsers that hasn’t yet come to their attention :wink:

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.