Simple validation script

Question

I'm playing about with JavaScript and wanted to create a simple validation script.

It works ok but is a bit clunky. How could I improve it?

var el = document.getElementById("username");
var el_pwd = document.getElementById("password");
var el2 = document.getElementById("feedback");
var el3 = document.getElementById("ok");
var el4 = document.getElementById("ok2");

function checkUsername() {
var username = el.value;
var password = el_pwd.value;
if((username.length < 5) & (password.length <= 0)) {
el2.className = 'warning'; 
el2.textContent = "Username Not long enough yet..";
el2.style.color = "red";
} else {

  el2.textContent = " ";
 }
}

function checkPassword() {
var username = el.value;
var password = el_pwd.value;
if((username.length >= 5) & (password < 7) ) {

el2.textContent = "Password MUST be 7 or more characters";
el2.style.color = "red";
} else if ((username.length <= 4) & (password.length <= 0))  {

el2.className = 'warning'; 
el2.textContent = "Username Not long enough yet..";
el2.style.color = "red";


 } else {
  el2.textContent = " ";
  }
}

function usernameOK() {
var username = el.value;
if(username.length >= 5) {
el3.style.display="block";
} else {
    el3.style.display = "none";
 }
}

function passwordOK() {
var password = el_pwd.value;

if(password.length >= 7) {
el4.style.display="block";
} else {
    el4.style.display = "none";
 }
}

function feedBack() {
el2.className = 'tip';
el2.textContent = "The username MUST be at least 5 characters";
el2.style.color = "blue";
}




el.addEventListener("focus", feedBack, false);
el.addEventListener("blur", checkUsername, false);
el.addEventListener("blur", usernameOK, false);

el_pwd.addEventListener("focus", checkPassword, false);
el_pwd.addEventListener("blur", passwordOK, false);

var el = document.getElementById("username");
var el_pwd = document.getElementById("password");
var el2 = document.getElementById("feedback");
var el3 = document.getElementById("ok");
var el4 = document.getElementById("ok2");

function checkUsername() {
  var username = el.value;
  var password = el_pwd.value;
  if ((username.length < 5) & (password.length <= 0)) {
    el2.className = 'warning';
    el2.textContent = "Not long enough yet..";
    el2.style.color = "red";
  } else {

    el2.textContent = " ";
  }
}

function checkPassword() {
  var username = el.value;
  var password = el_pwd.value;
  if ((username.length >= 5) & (password < 7)) {

    el2.textContent = "Password MUST be 7 or more characters";
    el2.style.color = "red";
  } else if ((username.length <= 4) & (password.length <= 0)) {

    el2.className = 'warning';
    el2.textContent = "Not long enough yet..";
    el2.style.color = "red";


  } else {
    el2.textContent = " ";
  }
}

function usernameOK() {
  var username = el.value;
  if (username.length >= 5) {
    el3.style.display = "block";
  } else {
    el3.style.display = "none";
  }
}

function passwordOK() {
  var password = el_pwd.value;

  if (password.length >= 7) {
    el4.style.display = "block";
  } else {
    el4.style.display = "none";
  }
}

function feedBack() {
  el2.className = 'tip';
  el2.textContent = "The username MUST be at least 5 characters";
  el2.style.color = "blue";
}




el.addEventListener("focus", feedBack, false);
el.addEventListener("blur", checkUsername, false);
el.addEventListener("blur", usernameOK, false);

el_pwd.addEventListener("focus", checkPassword, false);
el_pwd.addEventListener("blur", passwordOK, false);

@import url(http://fonts.googleapis.com/css?family=Oswald);
 body {
  font-family: 'Oswald', 'Futura', sans-serif;
  margin: 0px;
  padding: 0px;
}
#feedback.warning {
  background-image: url('https://cdn2.iconfinder.com/data/icons/freecns-cumulus/32/519791-101_Warning-128.png');
  background-repeat: no-repeat;
  background-size: 20px 20px;
  padding-left: 30px;
}
#feedback.tip {
  background-image: url('http://bibliomancy.org/images/icons/QuestionMark.png?1347492574');
  background-repeat: no-repeat;
  background-size: 20px 20px;
  padding-left: 30px;
}
#ok {
  position: absolute;
  top: 5px;
  left: 250px;
  background-image: url('http://upload.wikimedia.org/wikipedia/commons/c/cc/Ambox_green_tick.png');
  background-repeat: no-repeat;
  background-size: 20px 20px;
  padding-left: 30px;
  height: 50px;
  width: 50px;
  display: none;
}
#ok2 {
  position: absolute;
  top: 30px;
  left: 250px;
  background-image: url('http://upload.wikimedia.org/wikipedia/commons/c/cc/Ambox_green_tick.png');
  background-repeat: no-repeat;
  background-size: 20px 20px;
  padding-left: 30px;
  height: 50px;
  width: 50px;
  display: none;
}

<form>
  Username:
  <input type="text" id="username">
  <br>Password:
  <input type="password" id="password">
</form>

<div id="feedback"></div>
<div id="ok"></div>
<div id="ok2"></div>

Answer 1

Validation gone wrong

Every piece of code you write should solve a problem. Every good piece of code you write should solve a problem so small it has nearly no dependencies. So often, you'll end up writing a lot of little pieces of code in order to create beautiful software.

Take for instance a Calculator. Instead of cramming everything in one huge function. There will probably be tons of little methods that do as much as possible without knowing to much. A 'sum' method will have 2 arguments passed in, and return the sum. It couldn't care less where they come from.

Validation the right way

What you actually needed was some code that uses a validator to validate your usernames and passwords.

But what is a validator?

At it's core, a valdiator needs 2 things. It needs an input, and it needs a strategy to validate that input. It might even have a chain of strategies.

A Validator

First of, let's define our validator:

function createValidator(input, strategy) {
    var input = input,
        strategy = strategy;

    return {
        passes : function() {
            return strategy(input);
        }
    }
}

Now we can create our validators, we can start creating our strategies. For instance a LengthValidator strategy:

function createLengthValidator(min, max) {
    var minLength = min,
        maxLength = max || Infinity;

    return function(data) {
        return data.length <= max && data.length >= min;
    }
}

To use it we would do:

var usernameValidator = createValidator(
    document.getElementById("username").value,
    createLengthValidator(5)
);
var passwordValidator = createValidator(
    document.getElementById("password").value,
    createLengthValidator(7)
);

And then our check* functions:

function checkUsername() {
    if ( !usernameValidator.passes() ) {
        var feedback = document.getElementById("feedback");
        feedback.className = 'warning'; 
        feedback.textContent = "Username Not long enough yet..";
        feedback.style.color = "red";
    }   
}

function checkPassword() {
    if ( !passwordValidator.passes() ) {
        var feedback = document.getElementById("feedback");
        feedback.className = 'warning'; 
        feedback.textContent = "Password MUST be 7 or more characters";
        feedback.style.color = "red";
    }
}

Handle only the bare minimum

Your function checkUsername relies on the password (hunk?) and your checkPassword also checks the userName (hunk?). But they handle different criteria.

Your functions add a class, text and CSS. Woah, that's a lot. Let's refactor that using events.

Defining how it rolls

Defining the problem is always the hardest step. But somehow, a lot of people tend to skip this step. Don't.

What we want is the following: We have an input-field. Every time the input loses focus (blur event) the input should be evaluated. If the given input is not correct, show an error message.

Our problem is defined in 3 sentences. Each sentence defines a smaller problem. The first part is easy:

<input type="text" name="user_name" id="username" />

The second part, a little harder. But still doable since we already have our validator:

var $username = document.getElementById('username');

$username.addEventListener('blur', function() {
    var usernameValidator = createValidator(
        $username.value,
        createLengthValidator(5)
    );
    if ( usernameValidator.passes() ) {
        //create a validatorPassed event
        var event = new CustomEvent('validatorPassed');
        $username.dispatchEvent(event);
    } else {
        //create a validatorFailed event
        var event = new CustomEvent('validatorFailed');
        $username.dispatchEvent(event);
    }
});

Wow, so much code. But why?

Here is why:

$username.addEventListener('validatorPassed', function() {
    var feedback = document.getElementById("feedback");
    feedback.textContent = "";
});

$username.addEventListener('validatorFailed', function() {
    var feedback = document.getElementById("feedback");
    feedback.className = 'warning'; 
    feedback.textContent = "Username Not long enough yet..";
    feedback.style.color = "red";
});

See how we have successful decoupled our code? Our validator now knows nothing about our html. It simply validates input. We then have 2 eventListeners that listen to the Validator and do html-editing accordingly.

Disclaimer: I wrote this code inside the text-editor and is written as an example. I don't expect you to go all the way as I have. But it gives you the idea ;) always keep track of that one rule:

First make it work, then make it fast and then make it nice.

Answer 2

Firstly, rename your variables to something more readable. It's entirely unintuitive what var e12 is supposed to be - try names like username_input or username_element. e12 could just be called feedback or warning_message

Your indentation makes the code kind of hard to follow visually; look up a style guide to see the correct javascript indentation, or follow the basics of:

function whatever(){
  console.log("This is in one block (function) so it is indented once.")
  if (bool) {
    console.log("This is in two blocks (function, if) so it is indented twice.")
  }
}

The line if((username.length >= 5) & (password < 7) ) { looks like it should probably have password.length.

You can use the ternary operator to change the body of usernameOK to:

var username = el.value;
el3.style.display = (username.length >= 5) ? "block" : "none"

You can do something similar for the body of passwordOK.