Question

I have written (and copied) a few lines of Javascript and it serves my purpose well. But I am trying to figure out a better way (cross-browser and better performance) of doing this. I copied the isInteger function from a friend but I do not understand why we are checking against a string value in the following condition:

if (((c < "0") || (c > "9"))) return false;

The above condition works fine but when I change it to check against number values, the functionality breaks. The input field starts accepting alphabetical characters. Here is how it looks when I change it:

if ((( c < 0 ) || ( c > 9 ) return false;

I have tried to comment out sections so that you can understand what's happening. Also are there any security holes in this code? I read that the 1innerHTML1 method can open some security holes and hence we need to perform a 'clean' operation with it. Hence I chose to use jQuery's .html method (I am new to JavaScript)

The page in question: http://thehotdeal.net/clients/wehtmlit/index.php?order/

$(document).ready(function() {
  var total = 0;
  function calcTotal() {
  /* fetching some values from PHP variables and then performing calculations.
    essentially this is multiplying number of pages by price per page
  */
  /* <![CDATA[ */
    var total_price_main_pages = ($("#pages").attr("value")) * (<?php echo $main_price; ?>),
    total_price_sub_pages = ($("#subpages").attr("value")) * (<?php echo $sub_price; ?>);
    /*  ]] > */
    $("input.calculate:checked").each(function() {
    // This happens for each checked input field
    // These are few additional otions available to the user. If selected then
    // the price stored in their "data" attribute is added to the total
      var value = $(this).attr("data");
      total += parseInt(value); 
    });
    total += (parseInt(total_price_main_pages)) + (parseInt(total_price_sub_pages));
    $("#total").html("Total: <strong>" + total + "</strong>");
  }
  // This happens when the page loads
  calcTotal();
  $("input.calculate").click(function() {
    total = 0;
    calcTotal();
  });
  // function to check if an input is positive number(s). returns true if [ 0 <= s <= 9 ]
  function isInteger(s) {
    var i;
    for (i = 0; i < s.length; i++) {
      var c = s.charAt(i);
      if (((c < "0") || (c > "9"))) return false;
    }
    return true;
  }
  // Checking the mainpage input (default value 1)
  // (valid value is greater than or equal to 1 and less than 10)
  $("#pages").keyup(function() {
    var page = $(this).val();
    // if user deletes the value in this input (blank)
    // then just display a warning message and do nothing
    if(page == ""){
      this.value = "";
      $("#pageError").html("Please enter a value equal or greater than 1.");
      return false;
    }
    // if value is less than or equal to zero then
    // then set 1 as the new value, remove the error message and call the calcTotal function
    else if(page <= 0){
      this.value =1;
      $("#pageError").empty();
      total = 0;
      calcTotal();
    }
    // check if value is not a positive integer by calling the isInteger function
    // if not a positive integer then set 1 as the new value,
    //remove the error message and call the calcTotal function
    else if(!isInteger(page)){
      this.value =1;
      $("#pageError").empty();
      total = 0;
      calcTotal();
    }
    // if value does not fall in any of the if statements i.e. value is acceptable
    // remove the error message and call the calcTotal function
    $("#pageError").empty();
    total = 0;
    calcTotal();
  });
  // check if value is not empty when user exits the input
  // if empty then set value as 1, remove error message and call calcTotal function
  $("#pages").blur(function() {
    var page = $(this).val();
    if(page == ""){
      this.value = 1;
      $("#pageError").empty();
      total = 0;
      calcTotal();
    }
  });
  // Checking the subpage input (default value 0)
  // (valid value is greater than or equal to 0 but less than 10)
  $("#subpages").keyup(function() {
    var page = $(this).val();
    if(page == ""){
      this.value = "";
      return false;
    } else if(!isInteger(page)){
      this.value = 0;
      total = 0;
      calcTotal();
    }
    total = 0;
    calcTotal();
  });
  $("#subpages").blur(function() {
    var page = $(this).val();
    if(page == ""){
      this.value = 0;
      total = 0;
      calcTotal();
    }
  });
});
Was it helpful?

Solution

i do not understand why we are checking against a string value in the following condition

Because c is a character (really, a 1-character string), since that's what String.charAt returns. That said, the isInteger function could be written much more simply using a regex:

function isPositiveInteger(s)
{
    return !!s.match(/^[0-9]+$/);
    // or Rob W suggests
    return /^\d+$/.test(s);
}

or you could take another approach: convert the string to a number, make sure it's positive, and make sure that the floor of the number is the same as the original (thus it's an integer):

function isPositiveInteger(s)
{
    var i = +s; // convert to a number
    if (i < 0) return false; // make sure it's positive
    if (i != ~~i) return false; // make sure there's no decimal part
    return true;
}

OTHER TIPS

What about..

if( +inputString > 0 ) {
}

if its just about finding out if the input is a positive integer. If you also don't want to allow numbers with decimal points / floating point values, you should do with with input field validation like

<input type="text" pattern="\d+" required/>

This tells the input field to only only numbers. The required flag is optional, if present it won't allow any submit button to continue unless all patterns are satisfied.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top