If statement/validation question

its driving me crazy

[Sub_Seven]

Hey guys, its been a while since I don't post on this side, I have a quick question, this code is working fine:

function formValidator2(){
valid = true;
if ((document.getElementById('secondphone').value != '' ) && (document.getElementById('secondphone').value.length <= 7 )){
alert ( "Test." );
valid = false;
}
return valid;
}

So that way I validate the second phone number only if not empty, technically I don't need this second phone number to be required but if the person still wants to provide it I need it to have a min length (so far so good). Now the problem is when I try to add a regular expression under a variable to also make this field numeric only, this is what I'm trying that by the way is not working hence my post:

function formValidator2(){
valid = true;
var numExpsecphone = /^[0-9]+$/;
if ((document.getElementById('secondphone').value != '' ) && (document.getElementById('secondphone').value.length <= 7 ) && (document.getElementById('secondphone').value.match(numExpsecphone))){
alert ( "Test." );
valid = false;
}
return valid;
}

I hope I'm clear enough to get an answer, so could anyone please point where is my ignorance trying to make my life a living hell? Thanks people :)

[Fotiman]

1. You probably want a "var" in front of "valid"
2. You can make this more efficient by storing the result of document.getElementById('secondphone') in a variable instead of calling getElementById over and over.
3. Try using RegExp.test instead of match.
4. I think you are testing for a valid secondphone, in which case I think your tests may be slightly incorrect. I would assume you want to verify:
A) The string is not empty
B) The string is AT LEAST 7 characters in length (you have this wrong)
C) The string contains only digits

However, the first test (string not empty) is redundant, as both the second and third conditions would handle that case, so I would get rid of the first check.
So, here is how I would do it:

function formValidator2(){ 
  var valid = false,
    secondphone = document.getElementById('secondphone'),  
    numExpsecphone = /^[0-9]+$/; 
  if ((secondphone.value.length >= 7 ) && (numExpsecphone.test(secondphone.value))){ 
    alert ( "Test." ); 
    valid = true; 
  } 
  return valid; 
} 

Hope that helps.

[Sub_Seven]

1. You probably want a "var" in front of "valid"

Isn't this supposed to work either way?

2. You can make this more efficient by storing the result of document.getElementById('secondphone') in a variable instead of calling getElementById over and over.

True, missed that...

3. Try using RegExp.test instead of match.

Still didn't work :(

4. I think you are testing for a valid secondphone, in which case I think your tests may be slightly incorrect. I would assume you want to verify:
A) The string is not empty
B) The string is AT LEAST 7 characters in length (you have this wrong)
C) The string contains only digits

Yes and no,

I want to make sure the field is numeric and has a length restriction only if the person typed something in the field, the can leave it blank if they choose as it is the second phone number.

So:

A) The string is not empty >>> The string CAN BE empty.
B) Where I leave phone numbers have 8 digits, so anything <= 7 would be invalid, but people can either type 8 or more as they could enter area code.
C) The string contains only digits >>> Yes, but it still doesn't work

However, the first test (string not empty) is redundant, as both the second and third conditions would handle that case, so I would get rid of the first check.

I get what you are saying but if I remove "(document.getElementById('secondphone').value != '' )" the script makes this field required and people can't leave it blank if they choose.

Thanks for the help :)

[Fotiman]

Isn't this supposed to work either way?

Without the var, you are creating a global variable. It will work... unless you end up tromping on some other script that creates a global variable with the same name. Basically, it's a bit sloppy to use globals, so good practice would be to always include var.

I want to make sure the field is numeric and has a length restriction only if the person typed something in the field, the can leave it blank if they choose as it is the second phone number.

Ok, but your test case is still incorrect. I think this should do what you're looking for:

function formValidator2(){ 
 var valid = false,
  secondphone = document.getElementById('secondphone'), 
  numExpsecphone = /^[0-9]+$/;
 if (secondphone.value.length == 0) {
  valid = true;
 }
 else if ((secondphone.value.length >= 8 ) && (numExpsecphone.test(secondphone.value))){ 
  valid = true; 
 }
 alert("Returning " + valid);
 return valid; 
} 

[edited by: Fotiman at 7:27 pm (utc) on Sep 30, 2010]

[Sub_Seven]

Ok, one quick question before I try this (and go to lunch), I noticed how you changed (from my original code) the value of the var valid from true to false, then after the if statements valid = true;

I point this out because my function formValidator2() is holding at least 40 other validation scripts and they're set the opposite to your suggestion so if the only way to make your code work is how you originally intended I will have conflicts with everything else I've written right?

[Fotiman]

The reason I did it that way is because when I'm thinking of "validation" I want to validate that the something behaves correctly, so I assume the default to be that it does not and then once it has proven that it does, I set it to valid. To go in the other direction requires a sort of double-negative thinking. In other words, rather than validating that a field meets certain conditions, you are validating that it does NOT meet certain conditions. I find this to be harder to maintain.

However, if that's the way you want to do it, you could change my example above to be:

function formValidator2(){ 
  var valid = true,
    secondphone = document.getElementById('secondphone'), 
    numExpsecphone = /^[0-9]+$/;
  if (secondphone.value.length == 0) {
    //valid = true;
  }
  else if ((secondphone.value.length >= 8 ) && (numExpsecphone.test(secondphone.value))){ 
    //valid = true; 
  }
  else {
    // The phone number is not blank, it's length is < 8 or not numeric
    valid = false;
  }
  alert("Returning " + valid);
  return valid; 
} 

Which could then be rewritten as:

function formValidator2(){ 
  var valid = true,
    secondphone = document.getElementById('secondphone'), 
    numExpsecphone = /^[0-9]+$/;
  if (secondphone.value.length != 0) {
    if ((secondphone.value.length < 8 ) || (!numExpsecphone.test(secondphone.value))){ 
      // The phone number is not blank, it's length is < 8 or not numeric
      valid = false; 
    }
  }
  alert("Returning " + valid);
  return valid; 
} 

Notice that the condition with the size of the field and the numeric check became an OR condition instead of an AND, and also notice that we have to add a NOT (!) operator to the numeric case. This is the sort of double-negative checking that referred to above, which I find to be more difficult to follow.

Note, in my previous post I mistakenly left a 7 in there, though you said phone numbers where you are contain 8 digits. I've correct that in this post and will edit the previous post now.

[Sub_Seven]

Wow Fotiman, you are a smart person, that fixed the problem.

I do follow the way you think about the negative checking, I would assume I never thought of it like that because I usually never write javascript from scratch my self unless it is a few lines. I'm gonna try to make a pause later on and see if I can make a little test form and write (from scratch) two validating scripts, one with the negative checking and one following your preference, that way I could possible understand the benefits of the latter, thanks so much for the help, I appreciated :)