Pretty basic Javascript problem I'm having

Toggle sidebar Toggle sidebar
Nebbers

Nebbers

Senior member
Jan 18, 2011
649
0
0
I'm sure this is not the most thrilling thing ever, but I'm trying to finish something from Javascript 4th edition by Don Gosselin, which is probably the shittiest textbook I've ever used. It came with a page of corrections for typos and it's ridiculous how many errors there are, which is just what people who are just learning JS want to deal with.

There doesn't seem to be any mention of problems with this particular section of code, though. The buttons at the bottom of the page are where the problem is -- the clear/delete magazine buttons work fine, but the Add Magazine does some wacky stuff and doesn't work right. The cause of this is over my head at this point, maybe it'll be a simple error to spot for someone more experienced.

Code: 
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
 "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>Gosselin Gazette Subscription Form</title>
<meta http-equiv="content-type" content="text/html; charset=iso-8859-1" />
<link rel="stylesheet" href="js_styles.css" type="text/css" />
<script type="text/javascript">
/* <![CDATA[ */ 
function checkForNumber(fieldValue) {
	var numberCheck = isNaN(fieldValue);
	if (numberCheck == true) {
		window.alert("You must enter a numeric value!");
		return false;
		}
	}
function confirmPassword() {
	if (document.forms[0].password_confirm.value != document.forms[0].password.value) {
		window.alert("You did not enter the same password!");
		document.forms[0].password.focus();
		}
	}
function noDelivery() {
	for (var i=0;i<document.forms[0].delivery.length;++i) {
		if (document.forms[0].delivery[i].checked == true)
			document.forms[0].delivery[i].checked = false;
			}
	}
function noSunday() {
	for(var i=0;i<document.forms[0].sunday.length;++i) {
		if (document.forms[0].sunday[i].checked == true)
			document.forms[0].sunday[i].checked = false;
			}
	}
function sameShippingInfo() {
	if (document.forms[0].elements[5].checked == true) {
		document.forms[0].name_shipping.value = document.forms[0].name_billing.value;
		document.forms[0].address_shipping.value = document.forms[0].address_billing.value;
		document.forms[0].city_shipping.value = document.forms[0].city_billing.value;
		document.forms[0].state_shipping.value = document.forms[0].state_billing.value;
		document.forms[0].zip_shipping.value = document.forms[0].zip_billing.value;
		}
	else {
		document.forms[0].name_shipping.value = "";
		document.forms[0].address.shipping.value = "";
		document.forms[0].city_shipping.value = "";
		document.forms[0].state_shipping.value = "";
		document.forms[0].zip_shipping.value = "";
		}
	}
function addMagazine() {
	if (document.forms[0].elements[31].value == "")
		window.alert("You must enter a magazine name.");
	
	else {
		if (document.forms[0].magazines.options[0] && document.forms[0].magazines.options[0].value == "none")
		document.forms[0].magazines.options[0] = null;
		var magazine = new Option();
		magazine.text = document.forms[0].elements[31].value;
		magazine.value = document.forms[0].elements[31].value;
		nextItem = document.forms[0].magazines.length;
		document.forms[0].magazines.options[nextItem] = magazine;
		document.forms[0].elements[31].value = "";
		}
	}
function deleteMagazine() {
	var selectedItem = document.forms[0].magazines.selectedIndex;
		if (selectedItem == -1)
			window.alert(
				"You must select a magazine name in the list.");
		else
			document.forms[0].magazines.remove(selectedItem);
		}
		
/* ]]> */
</script>
</head>

<body>
<h1>Gosselin Gazette Subscription Form</h1>
<h2>Customer Information</h2>

<form action="FormProcessor.html" method="get" enctype="application/x-www-form-urlencoded">
<table border="0">
	<tr>
	<td valign="top"><h3>Billing Information</h3>
	<p>Name<br />
	<input type="text" name="name_billing" size="50" /></p>
	<p>City, State, Zip<br />
	<input type="text" name="city_billing" size="34" />
	<input type="text" name="state_billing" size="2" maxlength="2" />
	<input type="text" name="zip_billing" size="10" maxlength="10" onchange="return checkForNumber(this.value);"/></p>
	<p><input type="checkbox" onclick="sameShippingInfo();" />Same Shipping Information</p>
	</td>
	<td valign="top">
	<h3>Shipping Information</h3>
	<p>Name<br />
	<input type="text" name="name_shipping" size="50" /></p>
	<p>Address<br />
	<input type="text" name="address_shipping" size="50" /></p>
	<p>City, State, Zip<br />
	<input type="text" name="city_shipping" size="34" />
	<input type="text" name="state_shipping" size="2" maxlength="2" />
	<input type="text" name="zip_shipping" size="10" maxlength="10" onchange="return checkForNumber(this.value);"/></p></td>
	</tr>
</table>
<p>Telephone</p>
<p>(<input type="text" name="area" size="3" maxlength="3" onchange="return checkForNumber(this.value);"/>)
	<input type="text" name="exchange" size="3" maxlength="3" onchange="return checkForNumber(this.value);"/>
	<input type="text" name="phone" size="4" maxlength="4" onchange="return checkForNumber(this.value);"/></p>
	<p>Enter a password that you can use to manage your subscription online:</p>
	<p><input type="password" name="password" size="50" /></p>
	<p>Type the password again to confirm it.</p>
	<p><input type="password" name="password_confirm" size="50" onblur="confirmPassword();" /></p>
	<h3>Delivery Rates</h3>
	<table border="0">
	<colgroup align="left" width="100" />
	<colgroup span="4" align="center" width="100" />
	<tr><th>&nbsp;</th>
	<th>4 weeks</th>
	<th>13 weeks</th>
	<th>26 weeks</th>
	<th>52 weeks</th></tr>
	<tr><td><strong>Mon-Sat</strong></td>
	<td><input type="radio" name="delivery" value="12.60" onclick="noSunday();" />$12.60</td>
	<td><input type="radio" name="delivery" value="40.95" onclick="noSunday();" />$40.95</td>
	<td><input type="radio" name="delivery" value="81.90" onclick="noSunday();" />$81.90</td>
	<td><input type="radio" name="delivery" value="156.00" onclick="noSunday();" />$156.00</td></tr>
	<tr><td><strong>Every Day</strong></td>
	<td><input type="radio" name="delivery" value="13.56" onclick="noSunday();" />$13.56</td>
	<td><input type="radio" name="delivery" value="44.07" onclick="noSunday();" />$44.07</td>
	<td><input type="radio" name="delivery" value="88.14" onclick="noSunday();" />$88.14</td>
	<td><input type="radio" name="delivery" value="159.74" onclick="noSunday();" />$159.74</td></tr>
	</table>
	<p><strong>Sundays only ($3.50 per week)</strong>
	<input type="radio" name="sunday" value="weekly" onclick="noDelivery();" />Bill me weekly
	<input type="radio" name="sunday" value="monthly" onclick="noDelivery();" />Bill me monthly</p>
	<p>Do you subscribe to any other newspapers?</p>
	<p><input type="checkbox" name="newspapers" value="nytimes" />New York Times<br />
	<input type="checkbox" name="newspapers" value="bostonglobe" />Boston Globe<br />
	<input type="checkbox" name="newspapers" value="sfchronicle" />San Francisco Chronicle<br />
	<input type="checkbox" name="newspapers" value="miamaherald" />Miami Herald<br />
	<input type="checkbox" name="newspapers" value="other" />Other</p>
	<p>Do you subscribe to any magazines?</p>
	<p>Magazine <input type="text" size="68" /></p>
	<p>
	<input type="button" value="Add Magazine" onclick="addMagazine();" style="width: 120px" />
	<input type="button" value="Delete Magazine" onclick="deleteMagazine()" style="width: 120px" />
	<input type="button" value="Clear List" onclick="document.forms[0].magazines.options.length = 0;" style="width: 120px" /></p>
	<p><select name="magazines" multiple="multiple" size="10" style="width: 500px">
	<option value="none">Enter the magazines you subscribe to</option>
	</select></p>
</form>

</body>
</html>

If anyone actually takes the time to solve this problem, I love you.
 
Cogman

Cogman

Lifer
Sep 19, 2000
10,286
147
106
Yikes, this thing is full of yuckyness.

First off, use descriptive variables everywhere.

document.forms[0].elements[31].value = "";

This is yucky. What is document.form[0].element[31]? In this particular case, that is the "Add Magazine" button, but how would you know that just looking at the javascript? You wouldn't, it provides no clues or hints to what it might be.

The solution? Variables, lots of variables. Hell, make them global. doing something like

var addMagazineBtn = document.form[0].element[31];

and then using it everywhere in your code will make your code 100% more readable and debug-able.

That being said, you shouldn't ever use something like document.form[0] etc. What if you want to add a new form to the page? what if you add another element? that will screw up the whole system. What works MUCH better is giving the elements class and id tags and then searching for those. Javascript supports a function called document.getElementById This function will go through and find the element with the id tag that matches the input. It is very fast and implemented in every browser. No magic numbers or random guessing.

To make javascript even more palatable, use a library like jQuery

BTW, the specific "my button disappears" bug is caused by this line
document.forms[0].elements[31].value = "";
 
D

DaveSimmons

Elite Member
Aug 12, 2001
40,730
670
126
Agreed, without IDs that code is very brittle. One field change or adding an extra form above it on the page will break everything.
 
Nebbers

Nebbers

Senior member
Jan 18, 2011
649
0
0
Just to be clear this isn't actually code I wrote at all -- it's copied straight from the book, and this particular chunk doesn't extend beyond this section, just need to fix that one button and that's it for here I believe.

I will definitely agree that it's not very easily read. I've messed around with jQuery stuff a bit on my own prior to this class (not really writing anything, just modifying plugins) and I actually followed it... much of this code from the book is just meaningless to me right now.

Thanks for pointing out the problem line though, I'll see if I can't get it working now. Just getting back home now to finish this.
 
Cogman

Cogman

Lifer
Sep 19, 2000
10,286
147
106
Nebbers said:
Just to be clear this isn't actually code I wrote at all -- it's copied straight from the book, and this particular chunk doesn't extend beyond this section, just need to fix that one button and that's it for here I believe.

I will definitely agree that it's not very easily read. I've messed around with jQuery stuff a bit on my own prior to this class (not really writing anything, just modifying plugins) and I actually followed it... much of this code from the book is just meaningless to me right now.

Thanks for pointing out the problem line though, I'll see if I can't get it working now. Just getting back home now to finish this.
Click to expand...

That's fine and all, just realize that this code is how javascript SHOULDN'T be written. The book is teaching bad habits (which makes me think that the author doesn't know that the hell he is doing). That is always a bad combination, someone that doesn't know how to write good code trying to teach others how to write code.
 
Nebbers

Nebbers

Senior member
Jan 18, 2011
649
0
0
Yeah... I actually e-mailed the teacher of this class prior to the start of the semester because I'd read a *lot* of bad reviews about the book on Amazon. She assured me there were "very few errors" and then we get a corrections sheet with a list of typos of ridiculous length... on top of stuff like this.

Frustrating that I learn more in far less time getting help from you guys on a forum, replying out of your own good will, than I learn from a class I'm paying for... all just to get a piece of paper that looks good on a job application later on.

*sigh*

Thanks though, really.
 
Cogman

Cogman

Lifer
Sep 19, 2000
10,286
147
106
:). I added to the negative amazon reviews. Hopefully, some teacher comes across my review and is deterred from making this their book of choice.
 
Nebbers

Nebbers

Senior member
Jan 18, 2011
649
0
0
Good deal. I have a feeling I'll be writing one of my own, as well as a lengthy response to this book at the end of the semester when they ask for my feedback in the exit poll.
 
You must log in or register to reply here.

TRENDING THREADS

COMPANY
RESOURCES
FOLLOW
Top Bottom