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

Andrew Dickens
Andrew Dickens
18,352 Points

Objects and Arrays best practice

Hi I've created a fictional stationary company to practice js objects and arrays, I"m pretty pleased with the results but wanted to check if this is the best/driest way of doing things?

Any feedback or suggestions would be great

Also is there any way I can place the array in a function or something to make the data entry easier Thanks Andrew

//object constructor//
function productEntry (imgSource, productName, productInfo, priceEach,  priceGroup ){
    this.imgSource      = imgSource;
    this.productName    = productName;
    this.productInfo    = productInfo;
    this.priceEach      = priceEach;
    this.priceGroup     = priceGroup;
}


//create array//
var productArray =  new Array ();

// use the object constructor to build array
productArray[0] = new productEntry( "img/some-image1.jpeg", "Wood Pencil", "It is a wooden pencil", 54, 765);
productArray[1] = new productEntry("img/some-image2.jpeg", "Biro Pen", "Good if you need a biro pen", 87, 578);
productArray[2] = new productEntry( "img/some-image3.jpeg", "Felt Tip", "It's a felt tip Pen", 43 ,578 );
productArray[3] = new productEntry( "img/some-image4.jpeg", "Crayon", "It's a crayon... nice", 52, 456 );
productArray[4] = new productEntry( "img/img/some-image5.jpeg", "Permanent Marker", "Good for grafitti", 43 ,2345 );


// use for to loop throught the array and append to the page//
for ( var i = 0; i<productArray.length; i = i +1){
    var imgPrint = productArray[i].imgSource;
    var namePrint = productArray[i].productName;
    var infoPrint = productArray[i].productInfo;
    var priceEachPrint  = productArray[i].priceEach;
    var priceGroupPrint = productArray[i].priceGroup;

    $(".product-container").append(
                '<div class="product-template product-pens" id="product001">' + 
                '<div class="product-img">' +
                '<img src=" ' + imgPrint + ' " alt="product-img">' +
                '</div>' +
                '<div class="product-info">' +
                '<h3>' + namePrint +'</h3> ' +
                '<p>' + infoPrint + '</p>' +
                '</div>' +
                '<div class="product-price">' +
                '<p>Price Each &sect;' + priceEachPrint +'</p>' +
                '<p>Price Group &sect;' + priceGroupPrint  +  '</p>' + 
                '<p>Any special offer</p>' +
                '<div class="button add">ADD TO BASKET</div>' +   
                '</div>')
}

4 Answers

Michael Liendo
Michael Liendo
15,326 Points

It's definitely clean and easy to look at. Would definitely benefit from some optimizations however. Some changes I would make, without using ES6 syntax:

//object constructor// 
//I think you're trying to create a closure but in this case the data is being manipulated, simply assigned, so I would just
     //use an object literal and skip this part 
function productEntry (imgSource, productName, productInfo, priceEach,  priceGroup ){
    this.imgSource      = imgSource;
    this.productName    = productName;
    this.productInfo    = productInfo;
    this.priceEach      = priceEach;
    this.priceGroup     = priceGroup;
}

//Using an object literal instead of the above function:
var product1 = {
    imgSource: "img/some-image1.jpeg",
    productName: "Wood Pencil",
    productInfo: "It is a wooden pencil",
    priceEach: 54,
    priceGroup: 765
};

var product2 = {
    //same format as above
};

//create array//
//var productArray =  new Array (); <--while it works, it's verbose...
var productArry = [];  //shorthand syntax that does the same thing

// use the object constructor to build array
//
//productArray[0] = new productEntry( "img/some-image1.jpeg", "Wood Pencil", "It is a wooden pencil", 54, 765);
//productArray[1] = new productEntry("img/some-image2.jpeg", "Biro Pen", "Good if you need a biro pen", 87, 578);
//productArray[2] = new productEntry( "img/some-image3.jpeg", "Felt Tip", "It's a felt tip Pen", 43 ,578 );
//productArray[3] = new productEntry( "img/some-image4.jpeg", "Crayon", "It's a crayon... nice", 52, 456 );
//productArray[4] = new productEntry( "img/img/some-image5.jpeg", "Permanent Marker", "Good for grafitti", 43 ,2345 //);

//Since all you *really* doing is just assigning stuff to the end of the array, we can simplify, by doing:
productArray.push(product1, product2, product3);

//down the road, you added another product, all we have to type to add is..
productArray.push(product4);

// use for to loop throught the array and append to the page//
//if you have 2,000 items, creating a length variable outside of the loop optimizes since you're only creating the variable once and
//  not everytime it iterates. Also note the syntactic sugar of replacing i = i +1 with i++ (does the same thing).
var productLength = productArray.length
for ( var i = 0; i<productLength, i++{
    var imgPrint = productArray[i].imgSource;
    var namePrint = productArray[i].productName;
    var infoPrint = productArray[i].productInfo;
    var priceEachPrint  = productArray[i].priceEach;
    var priceGroupPrint = productArray[i].priceGroup;

  //When using jQuery and appending many elements to the screen, document fragments are your friend!
  //Instead of below, which adds each element, one by one to the DOM..
                '<div class="product-template product-pens" id="product001">' + 
                '<div class="product-img">' +
                '<img src=" ' + imgPrint + ' " alt="product-img">' +
                '</div>' +
                '<div class="product-info">' +
                '<h3>' + namePrint +'</h3> ' +
                '<p>' + infoPrint + '</p>' +
                '</div>' +
                '<div class="product-price">' +
                '<p>Price Each &sect;' + priceEachPrint +'</p>' +
                '<p>Price Group &sect;' + priceGroupPrint  +  '</p>' + 
                '<p>Any special offer</p>' +
                '<div class="button add">ADD TO BASKET</div>' +   
                '</div>'
  //It can be written as:
//create a container
   var documentFragment = $(document.createDocumentFragment());
//loop as usual
    for(var i=0; i < productLength; i++) {
       var imgPrint = productArray[i].imgSource;
        var namePrint = productArray[i].productName;
        var infoPrint = productArray[i].productInfo;
        var priceEachPrint  = productArray[i].priceEach;
        var priceGroupPrint = productArray[i].priceGroup;

//add everything to the fragment by creating a 'productSheet' variable
    var productSheet = $('<div class="product-template product-pens" id="product001">' + 
                '<div class="product-img">' +
                '<img src=" ' + imgPrint + ' " alt="product-img">' +
                '</div>' +
                '<div class="product-info">' +
                '<h3>' + namePrint +'</h3> ' +
                '<p>' + infoPrint + '</p>' +
                '</div>' +
                '<div class="product-price">' +
                '<p>Price Each &sect;' + priceEachPrint +'</p>' +
                '<p>Price Group &sect;' + priceGroupPrint  +  '</p>' + 
                '<p>Any special offer</p>' +
                '<div class="button add">ADD TO BASKET</div>' +   
                '</div>');
//All the stuff in the product sheet is wrapped nicely in a variable and only appended once
     documentFragment.append(productSheet);
   }//End for-loop
//Now when we touch the DOM by appending items, we're only adding one item (filled with all the stuff as before)
    $('.product-container').append(documentFragment);
Andrew Dickens
Andrew Dickens
18,352 Points

Hey Michael thanks so much this is exactly what I needed, some great tricks to tighten up the code especially .createDocumentFragment() which I'd never even come across. Have a good weekend Andrew

Andrew Dickens
Andrew Dickens
18,352 Points

...oh and excellent use of the word 'verbose'

Michael Liendo
Michael Liendo
15,326 Points

No problem man. Have to give you kudos for asking someone to review your code. I stress to so many people, "Good code writers, read more code than they create."

Andrew Dickens
Andrew Dickens
18,352 Points

So i entered the new code and everything is working fine, only I had .on("click", ) on the button but now it's not working. I guess it can't see the elements now they are fragments, so how can i select it?

$(".add").on("click", function(){
    var $thisProductHtml = $( this ).siblings( ".product-info" );
    var $thisProductName = $( $thisProductHtml ).children("h3").text();

    console.log( $thisProductName );   
    $(".basket-drop").append("<p>" + $thisProductName + "</p>" );
    }
);
Michael Liendo
Michael Liendo
15,326 Points

Actually, the items still get rendered to the DOM as expected (this can be seen in Chrome's developers tools). If you wanted to upload to codepen and send me a link, I'd be happy to take a look.

Andrew Dickens
Andrew Dickens
18,352 Points

Hi Michael here's the link http://codepen.io/andrewcodes404/pen/yawrwQ cut down a bit for this specific problem, the full version is here https://github.com/andrewcodes404/eco-pencil Thanks for the help