Need help in debugging the script

[sharapov]

Hello,

I need your help in debugging my script. Take a look at the code in action here: http://home.pacbell.net/sharapov/example.htm

The function of the script is to calculate values in the input text fields. Everything works fine until user replaces any value in the "Price" column with something like 0.03 (i.e. three cents), the script then automatically converts 0.03 to 0.30. Also if user replaces any value in "Price" Column with 2.05 the script will convert it to 2.04! Can somebody help me to fix this.

Thanks.

[khalidali63]

Use JavaScript function Number.toFixed() for decimal point ranges.

http://devedge.netscape.com/library/manuals/2000/javascript/1.5/reference/

Cheers

Khalid

[Nedals]

I got somewhat confused by your script, so I took the liberty of rewriting it. It works just as your does but without the problems

// my script.......
var pfrm = null;
var rows = null;
function init() {
// set the elemObj
var elemAry = new Array()
pfrm = document.pricesform;
rows = (pfrm.elements.length - 3)/5;
for (var i=1; i<rows; i++) {
// quantity fields
elemAry[(i*5)+2] = pfrm.elements[(i*5)+2];
elemAry[(i*5)+2].onblur = endRestrict;
elemAry[(i*5)+2].onfocus = startRestrict2;
elemAry[(i*5)+2].onchange = compute;
// price fields
elemAry[(i*5)+3] = pfrm.elements[(i*5)+3];
elemAry[(i*5)+3].onblur = endRestrict;
elemAry[(i*5)+3].onfocus = startRestrict1;
elemAry[(i*5)+3].onchange = compute;
}
compute();
}

function compute() {
// format and compute totals
var totQty = 0;
var totPrice = 0;
var total = 0;
for (var i=1; i<rows; i++) {
qty = parseInt(pfrm.elements[(i*5)+2].value);
totQty += qty;
price = pfrm.elements[(i*5)+3].value.replace(/\$/,""); // remove the '$'
price = (price-0).toFixed(2); // format to 2 decimal places
totPrice += price-0;
pfrm.elements[(i*5)+3].value = "$"+price; // write back to field
extPrice = (qty*price).toFixed(2);
total += extPrice-0;
pfrm.elements[(i*5)+4].value = "$"+extPrice; // write back to field
}

// Add totals. No formatting required. It's done above.
pfrm.quantity.value = totQty;
pfrm.price.value = "$"+totPrice;
pfrm.extprice.value = "$"+total;
}
// end my script

var textfocus = 0;
function startRestrict1() { textfocus = 1; }
function startRestrict2() { textfocus = 2;}
function endRestrict(){ textfocus = 0;}

function keyLogger(){
var k = event.keyCode;
if (textfocus == 1) {
if (((k<48)||(k>57))&&(k!=46)&&(k!=36)) return false;
if ((k==46)&&(event.srcElement.value.indexOf('.') != -1)) return false; // if more than one '.'
if ((k==36)&&(event.srcElement.value.length != 0)) return false; // if more than one '$'
}
if (textfocus == 2) {
if ((k<48)||(k>57)) return false; // <0 or >9
}
}

onload = init;
//window.onload=addPrices;
window.document.onkeypress=keyLogger;

[sharapov]

nedals,

Thank you for your responce. Your code is definetly less compless than mine. :). However, there are still minor bugs in it. Take a look at the code in action here:

http://home.pacbell.net/sharapov/example3.htm

I wrote some of the bugs that I found below the code. Can you take a look and tell me how can I eleminate those.

Thanks.

[Nedals]

That's too many errors on my part :(

Change these two lines as shown to fix problems 1 and 2

// Add totals. No formatting required.??? YES IT IS.......!!
pfrm.quantity.value = totQty;
pfrm.price.value = "$"+totPrice.toFixed(2);
pfrm.extprice.value = "$"+total.toFixed(2);

Add these two lines to fix problem 3
for (var i=1; i<rows; i++) {
qty = parseInt(pfrm.elements[(i*5)+2].value);
if (!qty || (qty == 0)) qty = 1;
totQty += qty;
pfrm.elements[(i*5)+2].value = qty;

Item 4 is a little more tricky. The script is activated by a change to the content of a field and changing from '0.00' to '0' is not considered a change, so nothing is done and you're left with 0. Let me think on this a little.

EDIT:
Got it.. I think!
Change these two lines as shown.
elemAry[(i*5)+2].onblur = compute; // endRestrict;
elemAry[(i*5)+3].onblur = compute; // endRestrict;

and add this as the last line in the compute function
textfocus = 0;

You may now get rid of the endRestrict function entirely
and these two lines
elemAry[(i*5)+2].onchange = compute;
elemAry[(i*5)+3].onchange = compute;
(after you've tested!)

[sharapov]

nedals,

Thank you very much for your responce once again. I made all of the changes that you have suggested and it looks that those cahnges fixed bugs that I had, but unfortunately (as it often happens) they created some other ones. :( If it is not to hard for you can you take a look on my sample page (http://home.pacbell.net/sharapov/example3.htm) and let me know how those bugs can be fixed. (I wrote new bugs that I found below the code on my sample page.)

Thanks in advance.

[Nedals]

1. What do you want the field to default to if only a '.' is entered.

2. The cursor positioning is caused by the onFocus that is used to control the 'textfocus', that in turn controls the keyboard entry. You, or I, will need a different technique to contol the keyboard entry (not onFocus()). Needs a little thought!!!

3. I missed the fact that your script added the ',' (currency formatting). The toFixed() method will not do that and you will likely need a seperate function (easiest way) to handle that. I may already have one. Let me look around.

Sure glad you get to do the testing :)

[sharapov]

nedals,

1. I would prefer if user wouldn't be able to place . (dot) in the field by itself at all. But if this is not possible, I guess if user manages to put . (dot) without anything else it should default to $0.00

Thank you for helping me out.

[Nedals]

The 5th line deals with '.' or '$.'. I added the 'var curr...' to keep the line length down
if (textfocus == 1) {
var curr = event.srcElement.value
if (((k<48)||(k>57))&&(k!=46)&&(k!=36)) return false;
if ((k==46)&&(curr.indexOf('.') != -1)) return false; // if more than one '.'
if ( (k==46) && ( (curr.length == 0) || (curr.length == 1 && curr == '$') ) ) return false; // '$.' or '.'
if ((k==36)&&(curr.length != 0)) return false; // if more than one '$'
}

Here's a function to add the ','.
function currency(amt) { // handles values upto $100,000,000.00. Big enough??
amt = (amt-0).toFixed(2);
if (amt.length > 10 ) { amt = amt.replace(/(\d+)(\d{6}.\d{2})$/g,"$1,$2") };
if (amt.length > 6 ) { amt = amt.replace(/(\d+)(\d{3}.\d{2})$/g,"$1,$2") };
return "$"+amt;
}

Add as a seperate function and edit the following 4 lines

pfrm.elements[(i*5)+3].value = currency(price); // write back to field
pfrm.elements[(i*5)+4].value = currency(extPrice); // write back to field
pfrm.price.value = currency(totPrice);
pfrm.extprice.value = currency(total);

I tried a few things re the cursor position but both the onFocus and onBlur cause the problem. If I get rid of them and do things a different way, other effects stop working. (mainly the 0.00 to 0 problem) . You may have to live with this unless some else can come up with a solution.

[sharapov]

nedals,

Thank you for all your help. The problem with the cursor moving to the front of the field is not a major one, so the user will just have to adobt. :)

Sorry to bug you again but there is a small bug in the last script update. This bug has to do with conversion of 10000 into 10,000. When you input the value into the text box, script converts it correctly, but if you click on the same text box again the script givea a NaN error.

Can you take a look at it here: http://home.pacbell.net/sharapov/example3.htm

Hopefully this is the last bug.
Thank you for all your help.

[Nedals]

Change this line...
price = pfrm.elements[(i*5)+3].value.replace(/\$/,""); // remove the '$'

to this..
price = pfrm.elements[(i*5)+3].value.replace(/\$|\,/g,""); // remove the '$' and ','

I'm using IE5.00 my the office and notice that it does not support the toFixed() method. You may not care, but I thought I'd mention it.

[sharapov]

nedals,

Well, I will use this script for the internal purposes, so I can control which browser the user will use. Most of our users have IE 5.5 or IE6.0. Thank you for letting me know, though.

By the way there is still a small bug in the quanform() function. when run for the value equal to a million it displays 1000,000.00 instead of 1,000,000.00.

I Fixed it by changing the line if (amt.length > 10 ) { amt = amt.replace(/(\d+)(\d{6}.\d{2})$/g,"$1,$2") };
into if (amt.length >= 10 ) { amt = amt.replace(/(\d+)(\d{6}.\d{2})$/g,"$1,$2") };


I tried to figure out what these (/(\d+)(\d{3}.\d{2})$/g,"$1,$2") values are but I couldn't find references anywhere.

Thanks.

[Nedals]

Originally posted by sharapov
I tried to figure out what these (/(\d+)(\d{3}.\d{2})$/g,"$1,$2") values are but I couldn't find references anywhere.

This is part of a regular expression
\d+ -> one or more decimal chars
\d{3} -> three decimal chars
. -> any char (this is an error even though it works) in should have been \. in all expressions
\. -> a period char
\d{2} -> two decimal chars
The 'g' at the end looks for repeated occurances. It doesn't need to be there. I put it there while testing another technique and forgot to take it out.

The (...) group sections of the expression which can now be retrieved using "$1,$2" (group1) comma (group2)

You will find all this better explained under 'regular expressions' in your javascript documentation. Once you learn how to use them, you'll find they can be very powerful.

[sharapov]

nedals,

Thank you for the explanation. Now I am begining to understand what those things were. :)

I tried to use newly acquired knowledge to edit Quantity field of the script, so that it would format the numbers in the same manner that it does for the "price" field (if user puts 1000 it would format it as 1,000). I wrote a separate function (see below), that should have done what I wanted. I also edited some lines in the script. However it worked half way. The value of the field changes to 1,000 if I put 1000, but as soon as I click on the other field the value changes to 10! Where did I mess up this time?

...
if (!qty || (qty == 0)) qty = 1;
totQty += qty;

pfrm.elements[(i*5)+2].value.replace(/\,/g,""); // remove ','
pfrm.elements[(i*5)+2].value = quanform(qty);

price = pfrm.elements[(i*5)+3].value.replace(/\$|\,/g,""); // remove the '$' and ','
...

function quanform(amt) {
amt = amt.toFixed(0);
if (amt.length > 6 ) { amt = amt.replace(/(\d+)(\d{6})$/,"$1,$2") };
if (amt.length > 3 ) { amt = amt.replace(/(\d+)(\d{3})$/,"$1,$2") };
return amt;
}

[Nedals]

qty = parseInt(pfrm.elements[(i*5)+2].value);
if (!qty || (qty == 0)) qty = 1;
totQty += qty;
pfrm.elements[(i*5)+2].value = qty;

quantity used to be an integer, but with the comma, it wont be.

qty = pfrm.elements[(i*5)+2].value;
// now get rid of the comma as you did
qty = pfrm.elements[(i*5)2].value.replace(/\,/g,"");
if (!qty || (qty == 0)) qty = 1;
totQty += (qty-0); // you might need the -0 to ensure qty is numeric and not a string.
// now you need to put the comma back for display. You don't need the toFixed(0) here because qty can ONLY be an integer.
pfrm.elements[(i*5)+2].value = quanform(qty);

[sharapov]

nedals,

I can't do pfrm.quantity.value = quanform(totQty);
to format total quantity, can I?

I have to do something like amt = (amt-0).toFixed();
before I ad commas. Right? I guess I'll add another function that will do that.

Thanks.

[Nedals]

I can't do pfrm.quantity.value = quanform(totQty);
Yes! you can BUT...., and this one got me going a bit :)

You wrote this function which would appear to be correct and is probably what I would have written (less the toFixed), but there's a litttttle problem. The function is designed to work with strings, not numbers, (since we are adding a comma). So edit as shown and it will work. (I took the liberty of changing amt to qty.

function quanform(qty) {
//amt = amt.toFixed(0);
qty = qty + ""; // convert qty to a string
if (qty.length > 6 ) { qty = qty.replace(/(\d+)(\d{6})$/,"$1,$2") };
if (qty.length > 3 ) { qty = qty.replace(/(\d+)(\d{3})$/,"$1,$2") };
return qty;
}

Let's hope that's it. :D