Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript JavaScript Objects Loop Through Objects Display an Array of Objects on the Page – One Solution

Ryan Miles
seal-mask
.a{fill-rule:evenodd;}techdegree
Ryan Miles
Full Stack JavaScript Techdegree Student 3,261 Points
Posted by Ryan Miles
.a{fill-rule:evenodd;}techdegree
Ryan Miles
Full Stack JavaScript Techdegree Student 3,261 Points

Please review my code, and tell me if I'm using "best practices"

Can someone tell me if I'm using best practices, or if there is anything I can change? I'd like to correct any bad habit before they become too ingraned. 

// Array of pets
let pets = [
  {name: "Aussie", type: "Dog", age: 7, photo: "aussie.jpg"},
  {name: "dachshund", type: "Dog", age: 7, photo: "dachshund.jpg"},
  {name: "persian", type: "cat", age: 7, photo: "persian.jpg"},
  {name: "pug", type: "dog", age: 7, photo: "pug.jpg"},
  {name: "tabby", type: "cat", age: 7, photo: "tabby.jpg"}
];

//html holder variable
let html = '';

//loop through the array of pets, to build the html string.
for(let i = 0; i < pets.length; i++){
  console.log(i);
 html += `<h2>${pets[i].name}</h2> <br> <h3>${pets[i].type}</h3> <br> <h3>${pets[i].age} years old</h3> <br> <img src=/img/${pets[i].photo}></img> <br><br>`;

}

//add the html to the page.
document.getElementById('demo').innerHTML = html;

2 Answers

Steven Parker
Steven Parker
229,732 Points
Steven Parker
Steven Parker
229,732 Points

Looks great from a code logic perspective, good job! :+1:

You can make a few formatting improvements:

  • add spacing after keywords and before block braces
  • keep indentation consistent and relative to nesting level
  • break up long lines
  • no blank line before block closure
for (let i = 0; i < pets.length; i++) {
  console.log(i);
  html += `<h2>${pets[i].name}</h2> <br> <h3>${pets[i].type}</h3> <br>
    <h3>${pets[i].age} years old</h3> <br> <img src=/img/${pets[i].photo}></img> <br><br>`;
}
Jason Larson
Jason Larson
8,353 Points

As Steven Parker said, the logic is fine, so it's not really a "best practice" issue, but more a matter of deciding on a styling standard. If you are working on a dev team, they should define these for you, or you should come up with some standards to use. For example, in some places, people would prefer to see object properties on individual lines, like this:

let pets = [
    {
        name: "Aussie",
        type: "Dog",
        age: 7,
        photo: "aussie.jpg"
    },
    ...
]

Also, some places would prefer to have html broken down like you would have it if it was in an html document, like:

html += `
    <h2>${pets[i].name}</h2> <br>
    <h3>${pets[i].type}</h3> <br>
    <h3>${pets[i].age} years old</h3> <br>
    <img src=/img/${pets[i].photo}></img> <br><br>
`;

Both ways are going to work the same. It's just a matter of readability and what the people you are working with are used to seeing, or how they prefer things to look. To get an idea of how some other places do it, you might look at some published style guides. Here is an article on the 5 Best JavaScript Style Guides

Gary A
Gary A
3,753 Points
Gary A
Gary A
3,753 Points

Remove the <br>elements!!!

<br> elements should RARELY be used in practice in favor of structured HTML, particularly when you're already putting things in heading elements.

If you need to add space below a heading element, that's what CSS is for. Adding a <br> element FORCES additional spaces... and your designers won't like you if you do that to them.

Additionally, I'd consider NOT hardcoding your image path into your code. It's far easier for someone to update a JS data file (and this will usually come from a data base).

Also, this is optional, but make it easier on yourself and less error prone, I personally would assign the pet to a simple variable like p. Usually it's bad practice to put single letter variables, but when you're in a simple loop of items like "pets" where it's blatantly obviously, it's fine. pets is a short words... but imagine something like playersStillActiveAndAwake. Assigning the current item to p or player is commonly done and make the code more readable. Compare the following and tell me which one is more readable and understandable?

playersStillActiveAndAwake = []; // imagine players in array
let p = playersStillActiveAndAwake[0];
let msg = `${p.name} has ${p.health} and ${p.pronoun} has ${totalTurns - p.currentTurn} turns left. 
Do you want to use your ${p.weapon}?`

let playersStillActiveAndAwake = []; // imagine players in array
let msg = `${playersStillActiveAndAwake[0].name} has ${playersStillActiveAndAwake[0].health} and 
${playersStillActiveAndAwake[0].pronoun} has ${totalTurns - playersStillActiveAndAwake[0].currentTurn} turns left. 
Do you want to use your ${playersStillActiveAndAwake[0].weapon}?`

As for the assignment, I'd write it like this:

for(let i = 0; i < pets.length; i++){
  let p = pets[I];
  html += `
  <h2>${p.name}</h2>
  <h3>${p.type}</h3>
  <h3>${p.age} years old</h3>
  <img src='${p.photo}' alt='${p.name}'></img>
`;

}