[code review] More efficient way?

JavaScript
1

The goal is to get the array of family members made and print out the results in the order created. Is there any way to tidy this up :)?

// Our Person constructor
function Person(name,age)
{
    this.name=name;
    this.age=age;
}

// Now we can make an array of people
var family=new Array();
family[0]=new Person("alice", 40);
family[1]=new Person("bob", 42);
family[2]=new Person("michelle", 8);
family[3]=new Person("timmy", 6);

// loop through our new array
for(i=0;i<family.length;i++)
{
    console.log(family[i].name);
}
2

There are some ways to tidy things up.

// Our Person constructor

This comment doesn’t explain anything that isn’t already known. Comments should not say what is happening, they should explain the purpose instead, where the code can not be made clear enough to provide such an explanation for you. Instead of explaining what the code is already fully capable of showing you from the code itself, it’s better to use comments to explain why things occur, or to provide a broad overview instead.

function Person(name,age)

Spacing. A space after the comma is best practice to help aid in readability.

{

Coding techniques from other programming languages, such as putting the curly brace at the start of the next line, is not just a bad idea - it can actually break JavaScript. Due to syntax limitations in JavaScript and how automatic semicolon insertion works, it is a fundamental principle in JavaScript that placing the curly brace at the start of a new line, is a practice that results in more fragile code.

    this.name=name;
    this.age=age;

Spacing before and after the equals sign is best practice to help improve readability of the code.

}

That line is good, it has no problems.

// Now we can make an array of people

Explaining what is happening once again, which is not a good use for comments.

var family=new Array();
family[0]=new Person("alice", 40);
family[1]=new Person("bob", 42);
family[2]=new Person("michelle", 8);
family[3]=new Person("timmy", 6);

It is preferred for the sake of consistency to use instead of new Array.
You can also create those new Person objects at the same time as defining the array, for example with:

It is also a common convention in JavaScript to use single quotes to delimit strings instead of double. Both work, but single provides more convenience than double.

var family = [
    new Person('alice', 40),
    new Person('bob', 42),
    new Person('michelle', 8),
    new Person('timmy', 6)
];

Due to the duplication that we see here though, it may be worth considering storing the name/age in an array, and using a loop to create the new Person objects instead. Especially if more people are planned to be added in further development.

// loop through our new array

Another useless comment.

for(i=0;i<family.length;i++)

The i variable has not been declared so it is going to be a global one instead. That’s a bad mistake. The best way to remedy that is to declare the i variable right at the start of the code, with the other var declaration.

Spacing should be added after the for word, which helps to reinforce that it is not a function name being called.
Also, spacing before and after the equals sign, after the semicolons, and before and after the less than sign for improved readability.


{

Should come after the closing parenthesis of the for statement, with a space separating the two.

    console.log(family[i].name);

Be aware that Internet Explorer doesn’t support console.log - there are shims though that help to provide support, such as https://github.com/kayahr/console-shim

}

No troubles with this line :slight_smile:

So what we end up with is something like the following:


// Create and show family info

function Person(name, age) {
    this.name = name;
    this.age = age;
}

var family = [
    new Person('alice', 40),
    new Person('bob', 42),
    new Person('michelle', 8),
    new Person('timmy', 6)
],
    i;

for (i = 0; i < family.length; i += 1) {
    console.log(family[i].name);
}

I have also replaced i++ with i += 1 because I agree with Douglas Crockford, that incrementors and decrementors are not as good to use as other techniques that make it clearer what is going on.
For some details, see http://stackoverflow.com/questions/971312/why-avoid-increment-and-decrement-operators-in-javascript

3

Comments-I didn’t create the comments. I just kept them there because of being lazy. So I’m ignoring any suggestions for that :). No offense to you.

As for readability, I prefer no spaces in stuff such as (name,age). It helps my readability.

I was mainly just asking for advice on how to shorten code, which you were able to answer with the array advice.

console.log is just the part of the program that I’m supposed to write. I mean, I’m using codecademy.com and they force me to write console.log. Though again, comments based on that don’t help me shorten the code.

Thank you Paul for the array advice :).

4

Another potential way to do that is to use the forEach method instead, which allows you to simplify things, and you won’t need that variable either.


family.forEach(function (person) {
    console.log(person.name);
});

The forEach method is available on all modern web browsers. If you want it to succeed on Internet explorer, the forEach documentation page has compatibility code that adds the capability for web browsers that don’t know how.

5

Wow, that’s some efficient looking code.

Thanks for the advice Paul.