3
\$\begingroup\$

I am working on an ad form for a local newspaper. I am not familiar with the programming side of things, still learning. I was able to successfully get the form to count the words, but I need it to quote the price based on different packages/ word counts. Here is an example of the form and what the price structure example is. Any help is appreciated to point me in the right direction! Please be nice, this is practice/learning for me!!

One paper $6.50 for 15 words, then 10 cents per word thereafter. Two papers $12.50 for 15 words then 15 cents per word thereafter. Three papers $18.00 for 15 words then 15 cents per word thereafter. Four papers $22.50 for 15 words then 15 cents per word thereafter.

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>Place An Ad</title>
<link href="adform.css" rel="stylesheet" type="text/css">
<script type="text/javascript" src="js/jquery-1.10.2.js"></script>
<script type="text/javascript">
counter = function() {
var value = $('#text').val();

if (value.length == 0) {
    $('#wordCount').html(0);
    return;
}

var regex = /\s+/gi;
var wordCount = value.trim().replace(regex, ' ').split(' ').length;
   

$('#wordCount').html(wordCount);
};

$(document).ready(function() {
$('#count').click(counter);
$('#text').change(counter);
$('#text').keydown(counter);
$('#text').keypress(counter);
$('#text').keyup(counter);
$('#text').blur(counter);
$('#text').focus(counter);
});
</script>
</head>

<body id="container">


<form>
<p>First Name<br>
<input type="text">
</p>
Last Name<br>

<input type="text">
<p>
Billing Address: <br><input type="text">
</p>
<p>
<input type="text">
</p>
<p>
City: <br><input type="text">
</p>
<p>
State: <br><input type="text">
</p>
<p>
Zip Code: <br><input type="text">
</p>
<p>
Classification: <br>
<select>
<option>4X4</option>
<option>Acreage</option>
<option>Antiques</option>
<option>Appliances</option>
<option>Apts Duplex For Rent</option>
<option>Auctions</option>
<option>Automobiles</option>
<option>Boats</option>
<option>Business for Sale</option>
<option>Business Opportunity</option>
<option>Childcare</option>
<option>Classic Vehicles</option>
<option>Commercial Rent</option>
<option>Commercial Sale</option>
<option>Computers</option>
<option>Electricians</option>
<option>Employment</option>
</select>
</p>
<p>Ad Body:<br><textarea id="text" cols="50" rows="4"></textarea>
<br>
<div id="result">
<strong>Words: <span id="wordCount">0</span> </strong><br />
<p>Attach Photo:<br>
<input type="file" name="pic" accept="image/*">
</p>
Number of Weeks: <br>
<select>
<option>1</option>
<option>2</option>
<option>3</option>
<option>4</option>
</select>
<p>
Extended Coverage Areas:<br>
Coverage area 1 <input type="checkbox" name="coverage" value="Coverage 1"> <sub>Eastern Washington including Spokane</sub><br>
Coverage area 2 <input type="checkbox" name="coverage" value="Coverage 2">    <sub>Lewiston and Moscow areas </sub><br>
Coverage area 3 <input type="checkbox" name="coverage" value="Coverage 3">   <sub>TriCities and surrounding areas </sub><br>
</p>
<p>Comments: <br>
<textarea cols="50" rows="3" id='text'></textarea>
</p>
<sub>Please note: Ads without complete billing information or ad text will    not be accepted.</sub>       
</form>
</body>
</html>

\$\endgroup\$
  • 2
    \$\begingroup\$ Congrats on learning to code! Note, though, that we can only review the code you posted (i.e. counting words). Guidance on code not yet written, such as calculating a price quote, is technically off-topic for CodeReview, I'm afraid. Can't review something that's not there yet. For such questions, head on over to StackOverflow. \$\endgroup\$ – Flambino Jun 18 '15 at 20:58
3
\$\begingroup\$

I'll add a simplification to your doc-ready event listeners. You can write it like this:

$(document).ready(function () {
    $('#count').click(counter);
    $('#text').on('change keydown keypress keyup blur focus', counter);
});

$.on can take multiple events separated by spaces.

\$\endgroup\$
2
\$\begingroup\$

There are a few simple things you should do first.

1) Move all of your script tag to the bottom of your html right before the closing </body> tag. Because of the way browsers parse your page, having them at the top will only slow down the page load.

2) You have two references to the jQuery library. You only need one.

3) Your HTML is not consistent. You have some open tags without closing tags. You also have different markup around some of the form fields than around the others. For instance:

<p>First Name<br>
  <input type="text">
</p>
Last Name<br>
<input type="text">

This could be written better and semantically like this:

<p>
  <label for="firstname">First Name</label><br/>
  <input type="text" placeholder="Ex: John" name="firstname" value="" />
</p>
<p>
  <label for="lastname">Last Name</label><br/>
  <input type="text" placeholder="Ex: Doe" name="lastname" value="" />
</p>

In this you have consistency around each input. They are given a name attribute which is needed when sending the data to the server. <label>s are used for labeling the input. Default placeholder text there to show the user what kind of input you are expecting, etc. By having this consistency it will make it easier to manipulate with code if you need to.

4) Always var your variables. You did this on all your variables except counter. Maybe just an oversight.

5) You should encapsulate your code in what is called an IIFE or Immediately Invoked Function Expression. It's a fancy name for creating yourself a private scope. This will help you avoid clashes between your code and code written by someone else. It's very easy to do:

(function( $ ) {
  // your code here
})( jQuery );

This is sorta of similar to saying:

function myFunc ( $ ) {
  //code here
}
myFunc( jQuery );

That's a simplified explanation. But you see we are passing in jQuery but we refer to it in our function as $.

6) In your document.ready function, you are creating all these events on the same element. You can simplify this as:

$('#text').on('change keyup blur focus', counter);

You don't need to specify keydown and keypress. By having all three, you are making the code get evaluated three times per key event.

7) As stated previously, naming your variables a little more descriptively will help you and any other developer later on. when you have to come back to this code in six months or a year, it will be much easier to figure out.

Congratulations on learning to code!

\$\endgroup\$
1
\$\begingroup\$

I'm going to make a few small tips, mostly about code style, and a few other things.

  • Indent your HTML, and your Javascript. Code is really hard to read if it isn't indented properly. Here's an example of how properly indented HTML/JS/CSS would look.

<!DOCTYPE html>
<html>
    <head>
        <style>
        ... {
            ...
        }
        </style>
        <script>
        function f() {
            ...
        }
        </script>
    </head>

    <body>
        ...
    </body>
</html>

  • Secondly, some of your naming is not the greatest. For example, what does the variable value do? Variable names should be descriptive and reflect on the purpose of the variable.
  • Finally, if you're using the latest version of JQuery, you don't need to include the following code. JQuery will work fine either way.

$(document).ready(function() {
    ...
});

\$\endgroup\$
  • \$\begingroup\$ Can you explain your last point? Why is the document.ready code not needed with the latest version of jQuery? As far as I know, if you need to manipulate DOM elements, then you need to wait for this event to occur. \$\endgroup\$ – Gary Storey Jun 19 '15 at 17:15
  • 1
    \$\begingroup\$ @GaryStorey See stackoverflow.com/questions/4643990/is-document-ready-necessary \$\endgroup\$ – Ethan Bierlein Jun 19 '15 at 17:58
  • \$\begingroup\$ Thanks for the link! In this case the OP's JavaScript is not at the bottom of the page. Also, in almost all of the answers (including the accepted answer), they so no it isn't required but do it anyway to prevent conflicts. \$\endgroup\$ – Gary Storey Jun 19 '15 at 18:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.