Reputation: 3
I'm looking for some help with a bit of code please. I'm supposed to create a form that allows the user to input a quantity for items which they wish to purchase. When the quantity is input, the total price for that particular item is displayed, and the grand total (at the bottom of the form) for all purchases. When the user presses the submit button, an alert popup appears.
I'm having trouble with the calculation part in javascript. It is not calculating any of the total amount or quantity values. (For some reason the code won't indent here properly but they are in the actual documents).
function calc(){
var QtyA = 0; var QtyB = 0; var QtyC = 0;
var TotA = 0; var TotB = 0; var TotC = 0;
var PrcA = 3; var PrcB = 4; var PrcC = 5.50;
if (document.getElementById('QtyA').value > "");{
QtyA = document.getElementById('QtyA').value;}
TotA = eval(QtyA) * eval(PrcA);
TotA = TotA.toFixed(2);
(document.getElementById('TotalA').value = TotA);
if (document.getElementById('QtyB').value > "");{
QtyB = document.getElementById('QtyB')value;}
TotB = eval(QtyB) * eval(PrcB);
TotB = TotB.toFixed(2);
(document.getElementById('TotalB').value = TotB);
if (document.getElementById('QtyC').value > "");{
QtyC = document.getElementById('QtyC')value;}
TotC = eval(QtyC) * eval(PrcC);
TotC = TotC.toFixed(2);
(document.getElementById('TotalC')value = TotC);
Totamt = eval(TotA) + eval(TotB) + eval(TotC);
Totamt = Totamt.toFixed(2); //fix to 2 decimal places
(document.getElementById('Grand Total is: ').value = Totamt);
alert (Totamt);
<html>
<head>
<meta charset="UTF-8">
<title>Order Form</title>
<style>
@import "css/OrderForm.css";
</style>
<body>
<form>
<table border="1">
<tr>
<th>Item</th>
<th>Image</th>
<th>Quantity</th>
<th>Price</th>
<th>Total</th>
</tr>
<tr>
<td width="80">Hat</td>
<td><img src="images/hat.jpg" alt="Hat"></td>
<td><input type="text" id="QtyA" size="5" onchange "calc()"></td>
<td>€3.00</td>
<td>
<input type="text" id="TotalA" size="12" onchange "calc()">
</td>
</tr>
<tr>
<td width="80">T Shirt</td>
<td><img src="images/t_shirt.jpg" alt="Hat"></td>
<td><input type="text" id="QtyA" size="5" onchange "calc()"></td>
<td>€4.00</td>
<td>
<input type="text" id="TotalA" size="12" onchange "calc()">
</td>
</tr>
<tr>
<td width="80">Glasses</td>
<td><img src="images/glasses.jpg" alt="Hat"></td>
<td><input type="text" id="QtyA" size="5" onchange "calc()"></td>
<td>€5.50</td>
<td>
<input type="text" id="TotalA" size="12" onchange "calc()">
</td>
</tr>
<tr>
<td> Total: </td>
<td><input type="text" id="GrandTotal" size="15" onchange="calc()"></td>
</tr>
</table>
<input type="submit" value="Submit">
<input type="reset" value="Reset">
</form>
</body>
</html>
Upvotes: 0
Views: 8324
Reputation: 65808
Ok, as a teacher, I just can't let all the bad habits that someone is trying to teach you go. So, here we go....
eval()
is EVIL - Don't use it, ever!
eval()
tells the JavaScript runtime to process a string as if it were JavaScript. This is very dangerous because if the string contains malicious code, eval()
will run it. In your code, you run eval()
on the value
entered into a text box and since you have no idea what value will be entered, you also have no idea what string eval()
will receive. This equates to a huge security hole and is one of the reasons eval()
should not be used. Secondly, even in a perfect world, eval()
is slow, so from a purely performance standpoint, you wouldn't want to use it. Frankly, I'm shocked that someone taught you to use it and especially for converting strings to numbers. That alone is enough to ask for your money back!
In your case, you need to convert the string input into numbers so that you can do math with the input. JavaScript offers several ways to do this:
parseInt(stringContainingNumber, radix)
parseFloat(stringContainingNumber)
Number(stringContainingNumber)
+stringThatIsNumber
Unary OperatorDon't set up your event handling in HTML with event attributes.
When JavaScript was first created (25+ years ago), the way to set up an event handler for an HTML element (a.k.a DOM element) was to use HTML attributes such as onclick
, onchange
, onmouseover
, etc. inline with the element in the HTML. Unfortunately, because of how simple that technique looks it gets used over and over again instead of dying the quick death it deserves. There are several reasons not to use this outdated technique. Today, we have modern standards and best practices to follow and so, event handling should be done in JavaScript, separate from HTML, with .addEventListener()
Also, your code of: onchange "calc()"
was incorrect anyway because the code should have been: onchange = "calc()"
.
Additionally, think about what elements need events set up for them. Your original code had it set up so that if the total gets changed, calc()
would run, but that makes no sense. Why would someone be able to change the total directly and what would doing so actually cause to happen? Should the quantity change because the total has changed?
Pay attention to details
You have 3 rows to calculate 3 quantities * 3 prices to get 3 totals, but you just copied/pasted the HTML for the 3 rows and wound up with 3 input elements with the same id
of QtyA
even though your JavaScript was correctly looking for QtyB
and QtyC
.
Do your styling with CSS, not HTML
All of your quantity input fields need to have their width set to a size of 5. Don't use the HTML size
attribute for that, do it with the width
CSS property. The HTML will be cleaner and you won't have to repeat the same instruction 3 times.
@import is being used incorrectly The CSS @import directive is meant to be used as the first line in external stylesheets that import instructions from another stylesheet, effectively combining multiple sheets into one. If you have only one stylesheet to use, you don't import it, you link to it.
Instead of: <style> @import "css/OrderForm.css";</style>
use: <link href="css/OrderForm.css" rel="stylesheet">
When you are just displaying a result, don't place it into a form field.
There's no reason to put a total into an input
field when you don't want the user to be able to modify that result. Instead, just place it as the text of a non-editable element - - in your case the appropriate cell of the table.
Lastly: Use the developer's tools! All modern browsers incorporate "developer's tools", which you can activate by pressing F12. There are many tabs in the tools, but the "Console" tab is probably the most important for you right now. If you have errors in your syntax (as you did), the Console will show them and the line number. You must eliminate all of your syntax errors before you can expect your code to run.
The Console is also an invaluable tool for testing values in your code. You can insert:
console.log(anything that is supposed to produce a value);
into your code to verify that variables, elements, etc. have the values you think they do.
Now, in reality, I would go about solving this problem in a very different way that you are attempting, but that is more complex than you are ready for at this stage, so I've gone along with your approach somewhat.
Please read through the HTML and JavaScript comments carefully to explain what's being done.
<!DOCTYPE html> <!-- The DOCTYPE tells the browser what version of HTML it should be expecting. -->
<html>
<head>
<meta charset="UTF-8">
<title>Order Form</title>
<!-- To reference a single stylesheet, use the link element: -->
<link href="css/OrderForm.css" rel="stylesheet">
<style>
/* Make all the input elements that have an id that starts with Qty
be 5 characters wide. (Now size=5 isn't needed in the HTML 3 times) */
input[id^=Qty] { width:5em; }
/* The first <td> in each row should be 80px wide. Now we don't have to
clutter up the HTML with this and we don't have to repeat it 3 times. */
td:first-child { width:80px; }
</style>
</head> <!-- You didn't close your <head> tag! -->
<body>
<form>
<table border="1">
<tr>
<th>Item</th>
<th>Image</th>
<th>Quantity</th>
<th>Price</th>
<th>Total</th>
</tr>
<tr>
<td>Hat</td>
<td><img src="images/hat.jpg" alt="Hat"></td>
<td><input type="text" id="QtyA"></td>
<td>€3.00</td>
<!-- You shouldn't be putting results of calculations into input fields
when you don't want the user to modify the data. Just place it into
an elmeent as its .textContent -->
<td id="TotalA"></td>
</tr>
<tr>
<td>T Shirt</td>
<td><img src="images/t_shirt.jpg" alt="T-Shirt"></td>
<td><input type="text" id="QtyB"></td>
<td>€4.00</td>
<!-- You shouldn't be putting results of calculations into input fields
when you don't want the user to modify the data. Just place it into
an elmeent as its .textContent -->
<td id="TotalB"></td>
</tr>
<tr>
<td>Glasses</td>
<td><img src="images/glasses.jpg" alt="Glasses"></td>
<td><input type="text" id="QtyC"></td>
<td>€5.50</td>
<!-- You shouldn't be putting results of calculations into input fields
when you don't want the user to modify the data. Just place it into
an elmeent as its .textContent -->
<td id="TotalC"></td>
</tr>
<tr>
<td> Total: </td>
<!-- You shouldn't be putting results of calculations into input fields
when you don't want the user to modify the data. Just place it into
an elmeent as its .textContent -->
<!-- You need to have this cell span over the remaining columns of the
table, so colspan=4 needs to be added. -->
<td id="grandTotal" colspan="4"></td>
</tr>
</table>
<!-- Your form doesn't actually submit data anywhere, so you shouldn't
have a submit button. A regular button will do. -->
<input type="button" value="Get Grand Total">
<input type="reset" value="Reset">
</form>
<script>
// Get references to the HTML elements that you'll be working with
var qtyBoxA = document.getElementById('QtyA');
var qtyBoxB = document.getElementById('QtyB');
var qtyBoxC = document.getElementById('QtyC');
var totBoxA = document.getElementById('TotalA');
var totBoxB = document.getElementById('TotalB');
var totBoxC = document.getElementById('TotalC');
var grandTot = document.getElementById('grandTotal');
var btnGetTot = document.querySelector("input[type=button]");
var btnReset = document.querySelector("input[type=reset]");
// Set up event handling in JavaScript, not HTML.
qtyBoxA.addEventListener("change", calc);
qtyBoxB.addEventListener("change", calc);
qtyBoxC.addEventListener("change", calc);
btnGetTot.addEventListener("click", getGrandTotal);
btnReset.addEventListener("click", reset);
var gt = null; // Will hold the grand total
function calc() {
var priceA = 3;
var priceB = 4;
var priceC = 5.50;
gt = 0;
// Convert the values in the quantity textboxes to numbers. The 10 that
// is being passed as the second argument indicates the "radix" or the
// numeric base system that should be used when the string is being
// interpreted. Here (and often), we work in the base 10 numeral system.
var qtyA = parseInt(qtyBoxA.value, 10);
var qtyB = parseInt(qtyBoxB.value, 10);
var qtyC = parseInt(qtyBoxC.value, 10);
// If each of the quantity fields are not empty, calculate the price * quantity
// for that row, place the answer in that row's total field and add the answer
// to the grand total
// NOTE: You had semicolons like this: if(); {}, which is incorrect.
// NOTE: Notice that there are + signs right in front of the total box references?
// this forces a conversion of the string in the text to a number. Since we
// just put a number into the cell, we know for sure it can be converted.
// NOTE: If parseInt() can't parse a number from the string provided, it returns NaN
// (Not A Number), we can check to see if we got NaN with the isNaN() function
// and here, we want to know if we don't have a NaN, so we prepend a ! to it
// (the logical NOT operator) to test the opposite of the isNaN() function result.
if (!isNaN(qtyA)) { totBoxA.textContent = qtyA * priceA; gt += +totBoxA.textContent; }
if (!isNaN(qtyB)) { totBoxB.textContent = qtyB * priceB; gt += +totBoxB.textContent; }
if (!isNaN(qtyC)) { totBoxC.textContent = qtyC * priceC; gt += +totBoxC.textContent; }
grandTot.textContent = gt.toFixed(2); // Just place the answer in an element as its text
}
function getGrandTotal(){
calc(); // Make sure all values are up to date
alert(gt);
}
function reset(){
// The built-in functionality of the <input type=reset> will clear out
// the quantity input fields automatically, but we need to manually reset
// non form field element that have been modified:
totBoxA.textContent = "";
totBoxB.textContent = "";
totBoxC.textContent = "";
grandTot.textContent = "";
}
</script>
</body>
</html>
Upvotes: 3