Hi,

I can't figure out why the condition I've highlighted in red below doesn't work. I'm checking to be sure that neither the "@" or the "." is the last character of the input. It functions properly when checking for the "@" but not the "." Even if I eliminate checking for the "@" and just check for the "." it still won't work.

function isValidEmail(email, required) {
    if (required==undefined) {   // if not specified, assume it's required
        required=true;
    }
    if (email==null) {
        if (required) {
            contact.email.focus();
            return false;
        }
        return true;
    }
    if (email.length==0) {  
        if (required) {
            contact.email.focus();
            return false;
        }
        return true;
    }
    if (! allValidChars(email)) {  // check to make sure all characters are valid
        contact.email.focus();
        return false;
    }
    if (email.indexOf("@") < 1) { //  must contain @, and it must not be the first character
        contact.email.focus();
        return false;
    } else if ((email.indexOf("@") || email.lastIndexOf(".")) == email.length) {  // @ or .  must not be the last character
        contact.email.focus();
        return false;
    } else if (email.lastIndexOf(".") <= email.indexOf("@")) {  // last dot must be after the @
        contact.email.focus();
        return false;
    }
	
    return true;
}

function allValidChars(email) {
  var parsed = true;
  var validchars = "abcdefghijklmnopqrstuvwxyz0123456789@.-";
  for (var i=0; i < email.length; i++) {
    var letter = email.charAt(i).toLowerCase();
    if (validchars.indexOf(letter) != -1)
      continue;
    parsed = false;
    break;
  }
  return parsed;
}

What is it that I'm missing here?

--Yizit

They have to be full statements. Right now your condition would read. "If '@', or if '.' not the last character".

else if ((email.indexOf("@") == email.length || email.lastIndexOf(".")) == email.length) {

As long as '@' was in the string, email.indexOf("@") would evalute to true.

Hi Chroder,

Hmmm... I understand what you are saying and I did make the changes you suggested but it still doesn't work. When the entry is xxx@xxx. (i.e. - with the dot at the end) it still returns true.

Also, I'm a bit confused by your last statement:

As long as '@' was in the string, email.indexOf("@") would evalute to true.

Isn't the evaluation more like: If '@' is the last character?

So far - Boolean=1 Me=0

--Yizit

Sorry, I was thinking indexOf returned false if the string was not found. Disregard that statement I should have said, email.indexOf("@") would always evalute to true (unless it's the first character in which it would be 0 and this, false). If the string is not found, indexOf returns -1, which still evaluates to true.

Isn't the evaluation more like: If '@' is the last character?

This is the main problem. The "OR" (||) operator is splitting the condition into two bits, it doesn't "extend" one into the other. Your condition is really saying (in most cases, excluding the '@' being first character like stated above) "If true, or if '.' is the last character" when you want it to say "If '@' is the last character, or if '.' is the last character".

With that said, I think the problem lies within the numbers. The length property holds the length of the string (obviously), but the indexOf() method returns the index of the string. The indexes start at zero, not one. For example, in the string "123", "1" would be in spot 0, "2" in spot 1 etc. So you need to alter it a bit further and subtract one from the length (or alternatively, add one to the indexOf):

else if ((email.indexOf("@") == email.length-1 || email.lastIndexOf(".")) == email.length-1) {

Hi Chris,

The length property holds the length of the string (obviously), but the indexOf() method returns the index of the string.

Thanks Chris! That's what was escaping me.

Now if I could only come up with a routine to prevent memory loss in my old age...

I appreciate your help!

--Yizit