Live price update with options

Question

I'm very new to JavaScript, so I'm not sure how to optimize the following code (which seems to run fine) further, though I did try to implement a few tips found on the web — such as using local variables for document elements, using do/while loops instead of for, or using join for string concatenation. I've read about using object literal instead of arrays, but I have to investigate further this point.

The script updates the displayed price on an HTML5 webpage when the client selects options. It is limited to radio, checkboxes and select input types. Each time one of these change, updatePrice() is called via onclick. The final price is to be updated, as well as discount thresholds (the more you order, the lesser the price).

Most variables are initialized from PHP (using <?php echo $opencart_variable; ?>, as for the HTML), hence the somehow unnatural "option[228]" form (it's not very relevant here but the script is intended as a (free) OpenCart extension). I'm open for suggestions on how to improve this, including switching to numbers, though I'm not sure I would implement them (in the "official" version of the extension, at least) since it may require large modifications of the theme and robustness of the extension depends on its intrusiveness.

var allOptionsValues = new Array(); // better solution ?
allOptionsValues["option[229]"] = new Array();
allOptionsValues["option[229]"]["21"] = 1.22;
allOptionsValues["option[229]"]["22"] = 0;
allOptionsValues["option[229]"]["23"] = 2.42;
allOptionsValues["option[228]"] = new Array();
allOptionsValues["option[228]"]["19"] = 1.22;
allOptionsValues["option[228]"]["20"] = 2.42;

var basicPrice = 12.14;
var currencyLeft = "";
var currencyRight = " €";
var decimalPoint = ",";

var discountThresholds = [];
var discountPrices = [];
discountThresholds.push(5);
discountPrices.push(9.14);
discountThresholds.push(10);
discountPrices.push(7.94);
discountThresholds.push(15);
discountPrices.push(6.74);
var discountText = " ou plus ";

var thisPage = document;
var quantity = thisPage.getElementById("input-quantity");
var optionsDiv = thisPage.getElementById("product");
var allInputs = optionsDiv.getElementsByTagName("input");
var allSelects = optionsDiv.getElementsByTagName("select");
var allDiscounts = thisPage.getElementsByClassName("discountPrice");
var priceList = thisPage.getElementsByClassName("finalPrice");

var nInputs = allInputs.length;
var nSelects = allSelects.length;
var nDiscounts = discountThresholds.length;
var nPrices = priceList.length;
var nProducts = 1;

var epsilon = 0.00001;

var newDiscountPrice = new Array(nDiscounts);
var newPrice = basicPrice;

function setPrices(delta)
{
    newPrice += delta;
    var i = nDiscounts;
    do {
        newDiscountPrice[i] += delta;
    } while (i--);
}

function updatePrice()
{
    // options prices will be added to (or removed from) the basic prices
    newPrice = basicPrice;
    var i = nDiscounts;
    do {
        newDiscountPrice[i] = discountPrices[i];
    } while (i--);

    // get number of products
    nProducts = quantity.value;

    // is a discount allowed for the current quantity ?
    for (i = 0; i < nDiscounts; ++i)
    {
        if (nProducts < discountThresholds[i])
            break;
        newPrice = discountPrices[i];
    }

    // loop on all input fields, update price and set quantity (assuming there is only one text/number field)
    i = nInputs - 1;
    do {
        var thisOpt = allInputs[i];
        if (thisOpt.checked)
            setPrices(allOptionsValues[thisOpt.name][thisOpt.value]);
    } while (i--);

    // loop on select fields, update price
    i = nSelects - 1;
    do {
        var thisSel = allSelects[i];
        if (thisSel.selectedIndex > 0)
            setPrices(allOptionsValues[thisSel.name][thisSel.options[thisSel.selectedIndex].value]);
    } while (i--);

    // update display of discount prices
    i = nDiscounts - 1;
    do {
        allDiscounts[i].innerHTML = [discountThresholds[i], discountText, currencyLeft, (newDiscountPrice[i]+epsilon).toFixed(2).replace('.', decimalPoint), currencyRight].join('');
    } while (i--);

    // update display of current price
    i = nPrices - 1;
    do {
        // priceList[i].innerHTML = [currencyLeft, (nProducts*newPrice+epsilon).toFixed(2).replace('.', decimalPoint), currencyRight].join(''); // faster than concatenation // TOTAL PRICE
        priceList[i].innerHTML = [currencyLeft, (newPrice+epsilon).toFixed(2).replace('.', decimalPoint), currencyRight].join(''); // faster than concatenation // PRICE PER UNIT
    } while (i--);

}

Answer 1

var allOptionsValues = new Array(); // better solution ?
allOptionsValues["option[229]"] = new Array();
allOptionsValues["option[229]"]["21"] = 1.22;
allOptionsValues["option[229]"]["22"] = 0;
allOptionsValues["option[229]"]["23"] = 2.42;
allOptionsValues["option[228]"] = new Array();
allOptionsValues["option[228]"]["19"] = 1.22;
allOptionsValues["option[228]"]["20"] = 2.42;

This looks... anyways. You can use PHP's json_encode to turn an associative array into JSON which in most cases, is valid JS object syntax:

// Don't know if short tags are still in fashion but whatever
var allOptionsValues = <?= json_encode($allOptionsValues); ?>;
// var allOptionsValues = {
//  "option[229]" : {
//    "21" : 1.22,
//    "22" : 0,
//    ... and so on
//  },
//  ... and so on
//}

So what the code did was turn $allOptionsValues into JSON and printed it directly on the page as allOptionsValues. But you have to check potential problems with this, such as what if $allOptionsValues isn't an array, what gets printed?

var discountThresholds = [];
var discountPrices = [];
discountThresholds.push(5);
discountPrices.push(9.14);
discountThresholds.push(10);
discountPrices.push(7.94);
discountThresholds.push(15);
discountPrices.push(6.74);

The same can be done to the above code. Here's an example, showing a hardcoded array being encoded for clarity:

// Is array() still "cool PHP"? Or is the [] notation "in" these days?
var discountThresholds = <?= json_encode(array(5, 10, 15)) ?>; // var discountThresholds = [5, 10, 15];
var discountPrices = <?= json_encode(array(9.14, 7.94, 6.74)) ?>; // var discountPrices = [9.14, 7.94, 6.74];

Same as the previous, check for edge cases and print defaults when necessary.

var basicPrice = 12.14;
var currencyLeft = "";
var currencyRight = " €";
var decimalPoint = ",";

These appears to be "constants" in the context of JS, and not variable. You might want to use the upper case and underscore convention for these:

var BASIC_PRICE = 12.14;
var CURRENCY_LEFT = "";
var CURRENCY_RIGHT = " €";
var DECIMAL_POINT = ",";

Answer 2

A few general recommendations that won't hurt performance.

Consistency

• Sometimes you use new Array and sometimes you use []. So far as I know the slightly preferred use is []. I am wondering why you are inconsistent? Seems best to choose one way of declaring a new array and stick to it unless you have an explicit reason for the different uses. If you do have an explicit reason for the different users, you should add a comment to explain that.

Commenting

• You break your variable declarations at the top into chunks, hopefully for logical reasons. Why not explain your reasons in short comments above each chunk? This will help future readers of your code and also help you know where to add new variables should the need arise.

Avoid global variables

• Javascript is no different from other languages in that it's generally better to avoid global variables and side effects where possible. In many of your functions, you access global arrays that easily could have been passed into and back out of the function. See e.g. setPrice and updatePrices. Consider using closures.