JavaScript-based product customization app

I'm developing a JS-based app that allows you to customize a product. I still consider myself a super newbie to javascript and especially to object oriented programming. So far, so good, the app does what I want it to do, so no trouble there.

My background is mostly in jQuery, so my main concern is how I'm sharing and reusing variables between functions, and how I can make this code prettier and more maintainable through optimizing those functions and vars.

The point of the app is:

• Click a product option

• Select a finish type

• Select a finish color

• If there is an additional price for that finish, update the price

I broke up what was once one big file into 3 modules and am loading them using LABjs:

$LAB
    .script(homeUrl + "/assets/js/product/product.js").wait()
    .script(homeUrl + "/assets/js/product/product-color.js")
    .script(homeUrl + "/assets/js/product/product-events.js")

PRODUCT.JS

```
(function (window, document, $) {
"use strict";

// Set some general element variables used throughout this script
var $nodes = {
list: $('.cust-list'),
steps: $('.steps'),
step: $('.cust-step'),
stepsSlide: $('.steps-slide'),
subtotal: $('.customizer .total .price'),
allTypes: $('.finish-type a'),
allColors: $('.finish-color a'),
options: $('.cust-option'),
checks: $('.cust-option-checklist a')
};

function Product (name) {
this.name = name;
this.options = [];
}

// Loops through the options slide divs
function optionsLoop($options, callback) {
for(var i = 0; i < $options.length; i++) {
callback(i, $options[i]);
}
}

// Loops through the array of product options within the product object
function productOptionsLoop(productOptions, callback) {
for(var i = 0; i < productOptions.length; i++) {
callback(i, productOptions[i]);
}
}

// Populate the product obje

Here are few things that I noticed with Product.js:

-
The Module design pattern is normally used to create private functions and variables. Unless if "use strict" is required, then there's no point in wrapping your code inside a closure when all the functions and variables are appended to the global namespace.

-
Variables shouldn't be passed to a function that won't get used. This is a problem in updateCost() for the variables addCost and totalPrice. This is also a problem for the document variable.

-
Try to only create varaibles for complex or redundant object references. So name and type aren't needed.

Previous code:

var $me = $(value),
    name = $me.attr('data-option'),
    type = $me.attr('data-option-type'),
    option = {
        option: name,
        type: type
    };
product.options.push(option);

New code:

var $me = $(value),
    option = {
        option: $me.attr('data-option'),
        type: $me.attr('data-option-type')
    };
product.options.push(option);

-
Instead of extending to the window, just add window. to the desired global variable or function name.

Example:

window.$nodes = {};

-
You should rename getProductOptions() to addOptionsToProduct() since the word get implies a return value.

-
This is redundant:

productPrice = +productPrice;
totalPrice = productPrice + addCost;

Just add productPrice to the end of a numeric operation for it to convert to a number:

totalPrice = addCost + productPrice;

-
Use existing functions instead of writing new ones. For example, take advance of jQuery.each and jQuery.map for interating through collections.

Example:

Previous code:

function productOptionsLoop(productOptions, callback) {
    for(var i = 0; i < productOptions.length; i++) {
        callback(i, productOptions[i]);
    }
}

New code (productOptionsLoop() is the same as below):

// if jQuery object
productOptions.each( callback );
// else
$.each( productOptions, callback );

-
Just add the cost, since 0 doesn't affect the value. But it would be even better if you got rid of addCost and used totalCost instead.

productOptionsLoop(productOptions, function(index,value){
    var $me = value,
        cost = $me.cost;
    if(value.cost) {
        addCost += value.cost;
    }
});

Becomes

productOptions.each(function(index,value){
    addCost += value.cost;
});

-
animateNumber() should call a callback once the animation part is complete. This way you don't need a setTimeout function.

Final code

// Set some general element variables used throughout this script
var $nodes = {
    list: $('.cust-list'),
    steps: $('.steps'),
    step: $('.cust-step'),
    stepsSlide: $('.steps-slide'),
    subtotal: $('.customizer .total .price'),
    allTypes: $('.finish-type a'),
    allColors: $('.finish-color a'),
    options: $('.cust-option'),
    checks: $('.cust-option-checklist a')
};
var Product = function(name) {
    this.name = name;
    this.options = [];
};
// Populate the product object with an array of options
var addProductOptions = function($nodes, product) {
    $nodes.options.each(function(index,value) {
        var $me = $(value),
            option = {
                option: $me.attr('data-option'),
                type: $me.attr('data-option-type')
            };

        product.options.push(option);
    });
};
// Change the cost according to the added options / variations
var updateCost = function(productOptions, $subtotal, productPrice, $nodes) {
    var totalPrice = productPrice, currentSubtotal = $subtotal.attr('data-subtotal');

    productOptions.each(function(index,value){
        totalPrice += value.cost;
    });
    animateNumber($nodes.subtotal, currentSubtotal, totalPrice, function(){
        $nodes.subtotal.text(totalPrice).digits();
    });
    $nodes.subtotal.attr('data-subtotal',totalPrice);
};
var updateOptions = function(productOptions, myOption, myName, myCost, myType) {
    // Go through the array of options and add the selected color and cost to this option
    productOptions.each(function(index,$this) {
        if($this.option !== myOption){
            return;
        }
        $this.name = myName;
        $this.cost = Math.floor(myCost);
        if(myType) {
            $this.type = myType;
        }
    });
};

StackExchange Code Review Q#15088, answer score: 3