Michael Grenzer
Michael Grenzer

Reputation: 501

Javascript if clause not working properly

I have the following Javascript/jQuery:

$('#List').keypress(function (e) {
  if (e.which == 13) {
    var lines = $('#List').val().split('\n');
    mylast = lines[lines.length - 1];
    mylen = mylast.length;
    if ((mylen != 8) || (mylen != 4)) {
      lines = lines.slice(lines.length-1);
      $('#List').val(lines.join("\n"));
      alert(mylen);
      return false;
    }
    return true;
  }
});

But it jumps into the code block after if even if length is 4 or 8....

Where is my error?

I want to remove the last line of the textarea if it's not of a given length.

Upvotes: -1

Views: 95

Answers (4)

Tomalak
Tomalak

Reputation: 338336

FWIW, here's a better version of your code:

$('#List').keypress(function (e) {
    var lines, last, valid;

    if (e.which === 13) {
        lines = $(this).val().split('\n');
        last  = lines.pop();
        valid = last.length === 4 || last.length === 8;

        if (!valid) {
            $(this).val(lines.join("\n"));
            e.preventDefault();
        }
    }
});
  • it declares all variables as local (never forget the var keyword!)
  • it uses $(this) instead of repeating the $('#List')
  • it uses e.preventDefault() instead of returning false
  • it uses a variable named valid to transport meaning clearer
  • it uses strict comparison (===)
  • if the code is part of a form validity check, consider putting it into the submit event handler instead of capturing the Enter key.

Upvotes: 0

zzzzBov
zzzzBov

Reputation: 179236

Lets examine the case of when mylen is 4.

mylen != 8 //true

mylen != 4 //false

true || false // true

What you probably want is to prevent mylen from being 8 and prevent mylen from being 4:

(mylen != 8) && (mylen != 4)

Upvotes: 0

T.J. Crowder
T.J. Crowder

Reputation: 1075337

This line:

if ((mylen != 8) || (mylen != 4)) {

means "if length is not 8 or length is not 4". So for instance, 4 is not 8, so the condition is true (the second part is never tested, because JavaScript expressions are short-circuited). Similarly, if it's 8, the first part of the expression is false, but the second part (8 != 4) is true, and so the expression is true.

You probably want:

if ((mylen != 8) && (mylen != 4)) {

&& means "and."

Upvotes: 0

Simon McLoughlin
Simon McLoughlin

Reputation: 8465

It should not be:

if ((mylen != 8) || (mylen != 4)) {

it should be:

if ((mylen != 8) && (mylen != 4)) {

your way if it was 8 , it was not 4 so it was getting through or if it was 4 it was not 8. you need to check that its not either

Upvotes: 2

Related Questions