Calculator for calculating price per feet amounts

Question

So I have a customers site I am remaking into an mvc3 app and they have an existing JavaScript pricing calculator. Is this safe and efficient? I am no JavaScript guy, but it looks pretty solid to me.

  <script type="text/javascript">

    window.onload = function () {

        //Carpet
        var ppf = .25; //price per square foot, edit this to suit
        //tile
        var ppfTile = .50; //price per square foot, edit this to suit
        var ppfVCTWood = .60; //price per square foot, edit this to suit
        var ppfTravertine = .60; //price per square foot, edit this to suit

        df = document.carpet;
        df[0].focus();
        df[2].onclick = function () {
            df[0].focus();
        };
        df[0].onkeyup = function () {
            if (isNaN(df[0].value)) {
                alert('numbers only please');
                df.reset();
                df[0].focus();
                return;
            }
            df[1].value = '$' + util.formatCurrency(df[0].value * ppf);
            df[2].value = '$' + util.formatCurrency(df[0].value * ppfTile);
            df[3].value = '$' + util.formatCurrency(df[0].value * ppfVCTWood);
            df[4].value = '$' + util.formatCurrency(df[0].value * ppfTravertine);
        };
    };
    var util = {
        formatCurrency: function (num) {
            num = isNaN(num) || num === '' || num === null ? 0.00 : num;
            return parseFloat(num).toFixed(2);
        }
    };
</script>

And here is the form.

<form name="carpet" action="#">
<div id="container">
    <h2 style="font-size: 12pt;">
        Cost Estimate</h2>
    <label>
        <input class="inp" type="text" />
        : Enter Square Footage</label>
    <br />
    <label>
        <input class="inp" type="text" value="$0" readonly="readonly" />
        : Estimated Cost For Carpet</label>
    <br />
    <label>
    <input class="inp" type="text" value="$0" readonly="readonly" />
    : Estimated Cost For Tile </label>
    <br />
    <label>
    <input class="inp" type="text" value="$0" readonly="readonly" />
    : Estimated Cost For VCT and Wood </label>
    <br />
    <label>
    <input class="inp" type="text" value="$0" readonly="readonly" />
    : Estimated Cost For Travertine Tile </label>
    <br />
    <input type="reset" />
</div>
</form>

I am worried this is old and outdated. Could I do this more effectively with C# in the app?

Answer 1

Here are a few thoughts. Some are probably overkill, but I'm just brainstorming.

Overall, it seems fairly solid if a little outdated. JohnP mentioned the bit about hard-coded prices, and I agree, but it's a question of maintainability rather than robustness.

I'd be concerned about directly assigning a function to window.onload. While it works, it also precludes you from adding other on-load functions later (you'd have to do some extra work at least), and/or you risk overwriting other onload handlers, or them overwriting yours. Libraries like jQuery will handle multiple event handlers for you, but if you want to avoid loading a library, there are still alternatives to attaching event handlers directly.

On the markup side, I'd give all the inputs proper IDs or at least name attributes, and refer to them that way in the JavaScript. The someform[nth_input] way of getting elements works, but it's pretty oldschool, impossible to understand without the markup, and easily breaks if you add more inputs to the form. Meanwhile something like document.getElementById("square_footage") is pretty self-explanatory. More verbose, yes, but much clearer.

I'd also consider using something besides input elements for the calculated prices. Technically, any element would work, and be eaiser to style than an input element. Since the fields are all readonly, they really aren't inputs anymore anyway. In your case, I'd probably use a table element (since it's monetary stuff) with ID'd td elements, and set their content with innerHTML.

The df[2].onclick = ... bit makes little sense to me. There are 4 fields with calculated values, so why does one in particular get a click-handler? Furthermore, the click handler makes it difficult for a user to (for instance) select the calculated price in that input and copy it. I just fail to see the point - seems like some old, left-over code.

Lastly, the util object ends up in the global scope, which is generally bad practice. Like window.onload you risk overwriting something or being overwritten by something. It doesn't need to be there, though. If formatCurrency is defined inside the onload function, it'll be available there without polluting the global scope.

Answer 2

About the only thing that I don't like in it is that you have to edit the code each time a price changes. I'd prefer that to come from a dynamic source, especially if you are going to use it in more than one spot.