views:

79

answers:

3

I have a small javascript pricing calculation that is not working on a friend's PC, but it works fine on my Mac. I'm guessing it's a semi-colon or something small that's not in the correct spot? Please help if you can?! The full page can be found @ http://procollage.com/pricing/photo-collage-pricing.html. Thank you all, again, in advance.

Here is the script.

<script LANGUAGE="JavaScript">
<!--

function calculate(PricingForm)
{
    height = eval(PricingForm.height.value);
    width = eval(PricingForm.width.value);
    photos = eval(PricingForm.photos.value);
    lgtext = eval(PricingForm.lgtext.value);
    mountlam = eval(PricingForm.mount.value);
    mountlam = eval(PricingForm.lam.value);

    GetPriceOne (PricingForm, height, width, photos, lgtext, mount, lam);
}

function GetPriceOne(PricingForm, height, width, photos, lgtext, mount, lam)
{
    PriceOne = height * width;
    GetPriceTwo(PricingForm, height, width, photos, lgtext, mount, lam, PriceOne);
}

function GetPriceTwo(PricingForm, height, width, photos, lgtext, mount, lam, PriceOne)
{
    PriceTwo = PriceOne / 144;
    GetPriceThree(PricingForm, height, width, photos, lgtext, mount, lam, PriceTwo);
}

function GetPriceThree(PricingForm, height, width, photos, lgtext, mount, lam, PriceTwo)
{
    PriceThree = PriceTwo * 15;
    GetPriceFour(PricingForm, height, width, photos, lgtext, mount, lam, PriceThree);
}

function GetPriceFour(PricingForm, height, width, photos, lgtext, mount, lam, PriceThree)
{
    if(PricingForm.lgtext.checked)
    {
        PriceFour = PriceThree + 20;
        GetPriceFive(PricingForm, height, width, photos, lgtext, mount, lam, PriceFour);
    }
    else
    {
        PriceFour = PriceThree;
        GetPriceFive(PricingForm, height, width, photos, lgtext, mount, lam, PriceFour);
    }
}

function GetPriceFive(PricingForm, height, width, photos, lgtext, mount, lam, PriceFour)
{
    if(PricingForm.mount.checked)
    {
        PriceFive = PriceFour + PriceTwo * 5;
        GetPriceSix(PricingForm, height, width, photos, lgtext, mount, lam, PriceFive);
    }
    else
    {
        PriceFive = PriceFour;
        GetPriceSix(PricingForm, height, width, photos, lgtext, mount, lam, PriceFive);
    }
}

function GetPriceSix(PricingForm, height, width, photos, lgtext, mount, lam, PriceFive)
{
    if(PricingForm.lam.checked)
    {
        PriceSix = PriceFive + PriceTwo * 5;
        GetPriceSeven(PricingForm, height, width, photos, lgtext, mount, lam, PriceSix);
    }
    else
    {
        PriceSix = PriceFive;
        GetPriceSeven(PricingForm, height, width, photos, lgtext, mount, lam, PriceSix);
    }
}

function GetPriceSeven(PricingForm, height, width, photos, lgtext, mount, lam, PriceSix)
{
    total = (photos * 4.95) + PriceSix;
    WriteDocument(total);
}

function RoundToPennies(n)
{
    pennies = n * 100;
    pennies = Math.round(pennies);
    strPennies = "" + pennies;
    len = strPennies.length;
    return strPennies.substring(0, len - 2) + "." + strPennies.substring(len - 2, len);
}

function WriteDocument(total) {
    document.PricingForm.collageEstimate.value = "$" + RoundToPennies(total);
}

//-->
</script>
+1  A: 

YUCK! Why on earth are you tippling through multiple functions?

GetPriceFive() calls PriceTwo which doesn't exist at that point. What you're trying to do is a complete bastardisation of programming. Here's a fresh start for you:

function calculate(PricingForm) {
    height = PricingForm.height.value;
    width = PricingForm.width.value;
    photos = PricingForm.photos.value;
    lgtext = PricingForm.lgtext.value;
    mountlam = PricingForm.mount.value;
    mountlam = PricingForm.lam.value;

    price = GetPrice(PricingForm, height, width, photos, lgtext, mount, lam)
    document.PricingForm.collageEstimate.value = "$" + RoundToPennies(price);
}

function GetPrice(PricingForm, height, width, photos, lgtext, mount, lam) {

        price = height * width;
        price = price / 144;
        pricetwo = price; // for lookup later
        price = price * 15;

        price = price + (PricingForm.lgtext.checked) ? 20 : 0;
        price = (PricingForm.mount.checked) ? price + pricetwo * 5 : price;
        price = (PricingForm.lam.checked) ? price + pricetwo * 5 : price;

        return (photos * 4.95) + price;
}

function RoundToPennies(n) {
    pennies = n * 100;
    pennies = Math.round(pennies);
    strPennies = "" + pennies;
    len = strPennies.length;
    return strPennies.substring(0, len - 2) + "." + strPennies.substring(len - 2, len);
}

That should do what your old code did (perhaps with a couple of oversights), without all the guff... And you have a lasting reference to the second price.

I dumped the write function because it wasn't needed. Functions that take up one line and are only called once don't need to be functions.

Oli
Thank you so much. I'm a COMPLETE newbie to Javascripting and know it's very much apparent. Your advise is much appreciated. I will try to rework it with what you suggested. I'm sure I'll have more questions, but I will post back. Thank you again.
dg
After looking at it, I clearly see what you're doing and it's AWESOME. It's all condensed into 1 function. I can't get it to work though 'cause I don't know enough...
dg
A fresh start wouldn't continue to use globals or eval.
David Dorward
I had to remove the evals. New JS developers, take heed.
keparo
Thank you again. You did it! See it working here for anyone interested: http://procollage.com/pricing/photo-collage-pricing.html.
dg
+1  A: 

It looks like you might need to do some reading before you try and tackle this problem. JavaScript will let you dig yourself quite a deep grave if you are not careful.

Douglas Crockford has some great videos on JavaScript.

http://video.yahoo.com/watch/111593/1710507

He's also written a fantastic book on the subject as well.

http://oreilly.com/catalog/9780596517748

Those should help you get well on your way to understanding the peaks and pitfalls of JavaScript. Good luck!

Stephano
A: 

using firefox on windows, the script blows up at some point during this:

height = eval(PricingForm.height.value);

it looks like one of the scripts/libraries that page is loading has commandeered the eval function. the script comes down on one line so it's not easy to debug. anyway stepping through the script doesn't make it to the next line.

and just a general pointer, i believe

var someUsefulName = document.getElementById('height');
var somethingElse = NoIdeaWhatThisMethodDoes(someUsefulName.value);

is preferred to the way you're accessing inputs on the page.

edit: why is the call to eval() in there? if it's meant to take in the value and derive something from it, replace eval() with an appropriate method. if you just want a copy of what's in the input, remove the calls to eval().

lincolnk