0
$('#save').click(function(){
    ($('input[type=text]').each(function () {
        if (isNaN($(this).val())) {
            alert("Some of your values were not entered correctly.");
            return 0;
        }
    });
});

I can't figure out why this isn't working. Any suggestions?

Edit: Sorry for the misleading title--I need to make sure the text boxes contain ONLY numbers.

Tim H
  • 205
  • 2
  • 17
  • They must contain only numbers, or they can contain the number anywhere in the text? Write some sample texts, that's how you can explain regex. – Shef Aug 30 '11 at 18:48
  • 1
    Yes: give us any error messages or warnings that you may be getting. Also, in `click(function(){(`, what is the last `(` and its matching `)` in `});` supposed to be doing? I believe those are not supposed to be there. – Piskvor left the building Aug 30 '11 at 18:50

2 Answers2

2

The isNaN function checks for a specific value called NaN[MDN], which is the result of trying to perform arithmetic operations on non-number objects. It doesn't check if a string is a number. How about a regular expression?

if (!/^\d+$/.test($(this).val())) {
    // Not a number...
}

The above will accept positive integers. If you need to accept negative or floating point numbers, change the regular expression to:

/^-?\d+$/         <-- Accepts positive and negative integers.
/^\d+(\.\d+)?$/   <-- Accepts positive integers and floating point numbers
/^-?\d+(\.\d+)?$/ <-- Accepts positive and negative integers and fpn.

Also, if you return 0 from within each, that stops the loop, but that doesn't prevent the click event from continuing on. You have invalid data, you need to call event.preventDefault() to halt the click event. Something like this will do:

$('#save').click(function(event){
    var badInputs = $('input:text').filter(function() { 
        return !$(this).val().match(/^\d+$/);
    };
    if (badInputs.length) {
        alert('Some of your values were not entered correctly.');
        event.preventDefault();
    }
});

Working example: http://jsfiddle.net/FishBasketGordo/ER5Vj/

FishBasketGordo
  • 22,904
  • 4
  • 58
  • 91
  • Do not use `:text`, because it will be handled by Sizzle. Besides your logic is wrong. – Shef Aug 30 '11 at 19:06
  • Corrected the logic mistake. I used the selector `input:text` because it will get inputs without a `type` attribute specified, which are rendered as textboxes. – FishBasketGordo Aug 30 '11 at 19:08
  • @FishBasketGordo: `input[type=text]` elements do not render as checkboxes. I don't get what you are talking about. Also, there is no need to iterate over all elements, the OP had it right with the breaking of the loop. – Shef Aug 30 '11 at 19:31
  • @Shef I'm not sure what *you* are talking about. I didn't say anything about checkboxes. As far as not iterating over all the elements, you're right, but in the original loop, the click handler would still proceed with a postback or what have you, when you really want it to stop right there so the user can correct the invalid data. If it means that much to you, set a flag before prematurely exiting the original loop and return false from the handler if it's set. The question is: what are you going to do with that extra half-millisecond you just saved? – FishBasketGordo Aug 30 '11 at 19:38
  • @FishBasketGordo: It's not about the half-millisecond, it's about teaching the right practice. I was talking about the Sizzle engine. An input element without a `type` attribute should not be used in the markup. Anyways, the OP makes the choice so it's up to him to decide what to take on and what not. Over and out. :) – Shef Aug 30 '11 at 19:40
0

in this topic a lot of dicussions about checking the numbers in JS. I hope it will help.

Community
  • 1
  • 1
Samich
  • 29,157
  • 6
  • 68
  • 77