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![](https://ecs-static.teamtreehouse.com/assets/views/marketing/shared/community-banner-white-47072046c51352fe6a69f5e691ff5700b28bb11d45197d7bdf066d9ea3f72d0c.webp)
![Ryan Miles](https://uploads.teamtreehouse.com/production/profile-photos/11658289/micro_huangshan.jpg)
Ryan Miles
Full Stack JavaScript Techdegree Student 3,261 PointsPlease 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](https://uploads.teamtreehouse.com/production/profile-photos/969492/micro_tool_chess_set_2.jpg)
Steven Parker
230,688 PointsLooks great from a code logic perspective, good job!
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>`;
}
![Gary A](https://secure.gravatar.com/avatar/b77de282ba07402a6d1285846d2bcc82?s=96&d=https%3A%2F%2Fecs-static.teamtreehouse.com%2Fassets%2Fcontent%2Fdefault_avatar-445fbbabfc8dc9188fb5967fe43322ee0c3e0dd1e10f378bf8343784af5a13eb.webp&r=pg)
Gary A
3,753 PointsRemove 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>
`;
}
Jason Larson
8,359 PointsJason Larson
8,359 PointsAs 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:
Also, some places would prefer to have html broken down like you would have it if it was in an html document, like:
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