Remove duplication to improve code

[Paul_Wilkins]

Using tests to rebuild the validate function is what we’re up to today.

Stepping back briefly to do thing properly

After the exciting wizardry of entirely removing the input handlers, today I take care of tests that should have been done yesterday.

Yesterday I was using existing tests for the input handlers to confirm that the validate function was behaving appropriately. But really, I should have created tests to define what the validate function needs to do.

Today I’ll do just that by recreating the validate function, using tests to define what it needs to do.

Check for empty value

The first thing that we want the validate function to do is to check for an empty value.

The this keyword in the input handler is not a jQuery object, and it refers to a form-group, so it helps if I start with a form-group reference.

And as the validate function uses inputStatus, we don’t need to check those details as they have already been checked elsewhere. We just need to check if the error message contains “is empty”. And when the value is not empty, to check if it doesn’t contain “is empty”.

describe("validate", function () {
    const emailGroup = $("#login .form-group").has("[name='E-mail']").get(0);
    const $emailError = $(emailGroup).find(".error");
    it("is empty", function () {
        emailGroup.value = "";
        validate(emailGroup);
        expect($emailError.html()).to.equal("E-mail is empty");
    });
});

We can tell the validate function to use checkEmpty,

function validate(inputGroup) {
    const value = $(inputGroup).find(".input-check").val().trim();
    checkEmpty(inputGroup, value);
}

and we are told exactly what to do to get things working.

Use test errors to create the checkEmpty() function

The test tells us: ReferenceError: checkEmpty is not defined at validate so we create a function.

function validate(inputGroup) {
    function checkEmpty() {
    }
    //...
}

Then the test says: AssertionError: expected 'Your email' to equal 'E-mail is empty' so we use inputStatus to achieve that. We also need to use inputGroup to target the warning at the right place.

    function checkEmpty(inputGroup) {
        inputStatus.warning(inputGroup, "E-mail is empty");
    }

And the test passes successfully.

Check for not empty

There’s no point in only checking for one type of thing. After all, a broken lightbulb is off all of the time, not only when the switch is turned off. We also want a test to check for what happens when the email isn’t empty. We should not find “is empty” in the error message.

    it("isn't empty", function () {
        emailGroup.value = "test.value@example.com";
        validate(emailGroup);
        expect($emailError.html()).to.contain("is Ok");
    });

The test now says: AssertionError: expected 'E-mail is empty' to not include 'is empty' telling us that the “isn’t empty” test shouldn’t see the empty warning. We need to check if the email is or isn’t empty, which means getting the value.

    function checkEmpty(inputGroup, value) {
        if (value === "") {
            inputStatus.warning(inputGroup, "E-mail is empty");
        }
    }

The next test message is: AssertionError: expected 'E-mail is empty' to not include 'is empty' so we need a way to say that everything is okay.

We could put the ok message in the checkEmpty function, but then we would also need to duplicate that in all other validation functions. That’s not a good idea.

Instead of doing that, we can use a separate function called showValid().

    function checkEmpty(inputGroup, value) {
        if (value === "") {
            inputStatus.warning(inputGroup, "E-mail is empty");
            return;
        }
        showValid(inputGroup);
    }
    function showValid(inputGroup) {
        inputStatus.ok(inputGroup, "E-mail is Ok: Your data has been entered correctly");
    }

It’s important to note that the test is not checking if “test value” results in ok. Other validations will clobber that kind of test.

Instead of of testing for empty or ok, safety of a sort is found by using the true dichotomy of is empty, or not empty. That way the test continues to be valid regardless of other checks that the code makes.

Check for fake text

The next test is for when we have fake text.

    it("is fake text", function () {
        input.value = "aaabbb";
        validate(emailGroup);
        expect($emailError.html()).to.equal("E-mail is Fake text: Please remove repetition");
    });
    it("isn't fake text", function () {
        input.value = "test.value@example.com";
        validate(emailGroup);
        expect($emailError.html()).to.contain("is Ok");
    });

We can add checkFake to our isValid code,

    checkEmpty(inputGroup, value);
    checkFake(inputGroup, value);

and the checkFake function to put in the validate function is easy to create:

    function checkFake(inputGroup, value) {
        const fakeReg = /(.)\1{2,}/;
        if (fakeReg.test(value)) {
            inputStatus.warning(inputGroup, "E-mail is Fake text: Please remove repetition");
            return;
        }
        showValid(inputGroup);
        return true;
    }

The email test now fails, because the checkFake is clobbering the checkEmpty function.

We need to find out if checkEmpty was successful or not. Returning true and false from the function is how we do that.

    function checkEmpty(inputGroup, value) {
        if (value === "") {
            inputStatus.warning(inputGroup, "E-mail is empty");
            return false;
        }
        showValid(inputGroup);
        return true;
    }
    //...
    const notEmpty = checkEmpty(inputGroup, value);
    if (notEmpty) {
        checkFake(inputGroup, value);
    }

Tidy up name and value

Now that the tests all pass, is a good time to refactor the code and make structural improvements.

I’ve been noticing that the value are being passed to several functions. Those functions can easily get the information from inputGroup, but I don’t want each function to use the $(inputGroup).find(".input-check").val().trim(); code to get the value. Instead, I’ll move the value code into a separate function instead.

function validate(inputGroup) {
    function getValue(inputGroup) {
        return $(inputGroup).find(".input-check").val().trim();
    }
    //...
    // const value = return $(inputGroup).find(".input-check").val().trim();
    const value = getValue(inputGroup);
    }```

We can now use getValue(inputGroup) in the functions, to help simplify the function parameters.

    // function checkEmpty(inputGroup, value) {
    function checkEmpty(inputGroup) {
        // if (value === "") {
        if (getValue(inputGroup) === "") {
            inputStatus.warning(inputGroup, "E-mail is empty");

and after removing value function parameters from all functions, we can simplify the end of the code too.

    // const value = getValue(inputGroup);
    // const notEmpty = checkEmpty(inputGroup, value);
    const notEmpty = checkEmpty(inputGroup);
    if (notEmpty) {
        // checkFake(inputGroup, value);
        checkFake(inputGroup);
    }

The single parameter functions are also easier to manipulate as an array, but we’ll get to that later on.

There is a problem with the above code though. When the value isn’t empty the inputStatus.ok() runs from the checkEmpty function, and then the code goes on to checkFake.

I don’t want inputStatus.ok() to run until after all of the checking functions have been attempted.

I would prefer that it inputStatus.ok() doesn’t occur until after all of the validations have succeeded.

For the notFake variable, we can check if notEmpty is true before using checkFake. That way we can check if both notEmpty and notFake are true before showing valid.

    const notEmpty = checkEmpty(inputGroup);
    const notFake = notEmpty && checkFake(inputGroup);
    if (notEmpty && notFake) {
        showValid(inputGroup);
    }

We can now remove that showValid from the checkEmpty and checkFake functions, and we have successfully extracted it from out of there.

We now don’t need the separate notEmpty and notFake variables. We can just use isValid, and work through with that instead.

    // const notEmpty = checkEmpty(inputGroup);
    // const notFake = notEmpty && checkFake(inputGroup);
    const isValid = checkEmpty(inputGroup) && checkFake(inputGroup);
    if (isValid) {
        showValid(inputGroup);
    }

That way, we can keep on adding more && sections to isValid for the other checks, and everything keeps on working fine.

Check is email

The test for checking if the value is an invalid or a valid email is similar to the other tests:

    it("is invalid email", function () {
        input.value = "test.value";
        validate(emailGroup);
        expect($emailError.html()).to.equal("E-mail is Incorrect: Please enter it correctly");
    });
    it("is valid email", function () {
        input.value = "test.value@example.com";
        validate(emailGroup);
        expect($emailError.html()).to.not.contain("is Incorrect");
    });

We can add checkEmailReg to the isValid line of code:

    // const isValid = checkEmpty(inputGroup) && checkFake(inputGroup);
    const isValid = checkEmpty(inputGroup) && checkFake(inputGroup) && checkEmailReg(inputGroup);

We can use the same structure of the checkEmpty and checkFake functions, for when creating the checkEmailReg function.

    function checkEmailReg(inputGroup) {
        const emailReg = /^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/;
        const value = getValue(inputGroup);
        if (!emailReg.test(value)) {
            inputStatus.warning(inputGroup, "E-mail is Incorrect: Please enter it correctly");
            console.log("isn't email");
            return false;
        }
        return true;
    }

That works, but improvement is needed.

Refactoring the isValid checks

The isValid line now has three functions chained together with && symbols. That is not a pattern that can successfully grow.

We really need to put those function names into an array, and loop through them getting the result instead.

    const emailTypes = [checkEmpty, checkFake, checkEmailReg];

But how are we going to efficiently work through them? We don’t want all of them to be checked when it’s not needed, as that can lead to multiple messages interfering with each other. We want an array technique that stops as soon as it gets a false result.

Looking at the list of array methods

In that article we find the Array.every method which says:

The every method executes the provided callback function once for each element present in the array until it finds the one where callback returns a falsy value. If such an element is found, the every method immediately returns false.

We can replace the isValid code now, using the every method.

    // const isValid = checkEmpty(inputGroup) && checkFake(inputGroup) && checkEmailReg(inputGroup);
    const isValid = emailTypes.every(function (check) {
        return check(inputGroup);
    });

In my previous post I used Array.some which stops when it gets a truthy value, whereas Array.every in this post stops when it gets a falsy value. It can be handy to know that Array.some and Array.every are the opposite of each other.

That’s the email code all dealt with. Now it’s on to passwords.

Test for empty password value

The first password test checks for an empty value.

To help keep the tests organised, I’ve put the email tests into their own describe section, so we can have a separate password tests section in there too.

describe("validate", function () {
    describe("email", function () {
        //...
    });
    describe("password", function () {
    });
});

The password test is as follows:

    describe("password", function () {
        const passwordGroup = $("#login .form-group").has("[name='Password']").get(0);
        const input = $(passwordGroup).find("input").get(0);
        const $passwordError = $(passwordGroup).find(".error");
        it("is empty", function () {
            input.value = "";
            validate(passwordGroup);
            expect($passwordError.html()).to.equal("Password is empty");
        });
        it("isn't empty", function () {
            input.value = "Password123";
            validate(passwordGroup);
            expect($passwordError.html()).to.not.contain("is empty");
        });
    });

There is a problem though - we are told by the test: AssertionError: expected 'E-mail is empty' to equal 'Password is empty'

Use field name to differentiate what gets checked

The email test is interfering with the password test, which is bad news. We need the email validation to restrict itself only to email fields.

As each input field has a reliably consistant name with it, we can use that name to differentiate between the different types of input fields, and move the emailTypes check into an if statment.

    function getName(inputGroup) {
        return $(inputGroup).find(".input-check").attr("name");
    }
    //...
    const emailTypes = [checkEmpty, checkFake, checkEmailReg];
    let isValid = false;
    if (getName(inputGroup) === "E-mail") {
        isValid = emailTypes.every(function (check) {
            return check(inputGroup);
        });
    }

I’m not happy about emailTypes configuration being low down in the function either. Things tend to be better when configuration is kept near the top of the function, where we can easily find it. I’ll move that emailTypes up to the top of the validate function.

function validate(inputGroup) {
    const emailTypes = [checkEmpty, checkFake, checkEmailReg];
    //...
    let isValid = false;

The password test now fails for the correct reason: AssertionError: expected 'Your password' to equal 'Password is empty'

Adding the password check

We can go ahead to add some password code to the validate function. Up where we have that emailTypes array, I’ll add a passwordTypes array too.

    const emailTypes = [checkEmpty, checkFake, checkEmailReg];
    const passwordTypes = [checkEmpty];

and we can see if the input field is a password one, before checking that field.

    let isValid = false;
    if (getName(inputGroup) === "E-mail") {
        //...
    }
    if (getName(inputGroup) === "Password") {
        isValid = passwordTypes.every(function (check) {
            return check(inputGroup);
        });
    }

We are given a test error of: AssertionError: expected 'E-mail is empty' to equal 'Password is empty' so we need to update the checkEmpty function so that it uses the correct name.

Use the name variable in the check functions

In the checkEmpty function we can use getName to get the appropriate name to use on the warning message:

    function checkEmpty(inputGroup) {
        if (getValue(inputGroup) === "") {
            inputStatus.warning(inputGroup, getName(inputGroup) + " is empty");

and the tests all successfully pass.

This is also a good time to update all of the other check functions to use getName(inputGroup) instead.

Improve the code structure

We will eventually have more than just email and passwords to check. There’s a huge set of requirements in the registration form that will eventually be taking place too.

All of the code in the if email and if password sections is nearly identical, all but for the emailTypes and passwordTypes variables. We can associate the names of “E-mail” and “Password” with emailTypes and passwordTypes by putting them into a validationTypes object. That way, we can get the types to use, and replace emailTypes and passwordTypes with just types, making the contents of the if statements exactly the same.

    // const emailTypes = [checkEmpty, checkFake, checkEmailReg];
    // const passwordTypes = [checkEmpty];
    const validationTypes = {
        "E-mail": [checkEmpty, checkFake, checkEmailReg],
        "Password": [checkEmpty]
    };
    //...
    const types = validationTypes[getName(inputGroup)];
    let isValid = false;
    if (getName(inputGroup) === "E-mail") {
        // isValid = emailTypes.every(function (check) {
        isValid = types.every(function (check) {
            return check(inputGroup);
        });
    }
    if (getName(inputGroup) === "Password") {
        // isValid = passwordTypes.every(function (check) {
        isValid = types.every(function (check) {
            return check(inputGroup);
        });
    }

Now that the if statements have exactly the same contents, we can remove the if statements as they’re no longer needed.

    const types = validationTypes[getName(inputGroup)];
    // if (getName(inputGroup) === "E-mail") {
    //     isValid = types.every(function (check) {
    //         return check(inputGroup);
    //     });
    // }
    // if (getName(inputGroup) === "Password") {
    //     isValid = types.every(function (check) {
    //         return check(inputGroup);
    //     });
    // }
    const isValid = types.every(function (check) {
        return check(inputGroup);
    });

The code that gets the types and uses them, is now easily organised away into a validateByTypes function.

    function validateByTypes(inputGroup) {
        const name = getName(inputGroup);
        const types = validationTypes[name];
        return types.every(function (check) {
            return check(inputGroup);
        });
    }
    let isValid = validateByTypes(inputGroup);

Check password for fake text

The next password test for fake text is as follows:

        it("is fake text", function () {
            input.value = "aaabbb";
            validate(passwordGroup);
            expect($passwordError.html()).to.equal("Password is Fake text: Please remove repetition");
        });
        it("isn't fake text", function () {
            input.value = "Password123";
            validate(passwordGroup);
            expect($passwordError.html()).to.not.contain("Fake text");
        });

After making improvements to the validate code, it’s really easy to make those tests pass.

    const validationTypes = {
        "E-mail": [checkEmpty, checkFake, checkEmailReg],
        // "Password": [checkEmpty]
        "Password": [checkEmpty, checkFake]
    };

Check for short password

The tests for a short password can follow the same structure that we’ve developed:

        it("is short password", function () {
            input.value = "ab";
            validate(passwordGroup);
            expect($passwordError.html()).to.equal("Password is Incorrect: Please enter at least 6 characters");
        });
        it("isn't a short password", function () {
            input.value = "Password123";
            validate(passwordGroup);
            expect($passwordError.html()).to.not.contain("enter at least");
        });

We add a reference to a checkPasswordShort function to the validator by updating the password rules:

        // "Password": [checkEmpty, checkFake]
        "Password": [checkEmpty, checkFake, checkPasswordShort]

The checkPasswordShort function to make the test pass is as easy as using the regular expression for a short password.

    function checkPasswordShort(inputGroup) {
        const pswReglow = /^([a-zA-Z0-9]{0,5})$/;
        const value = getValue(inputGroup);
        if (pswReglow.test(value)) {
            inputStatus.warning(inputGroup, getName(inputGroup) + " is Incorrect: Please enter at least 6 characters");
            return false;
        }
        return true;
    }

Check for long password

The final test for a long password are almost anticlimatic, as they are similar to the test for a short password:

        it("is long password", function () {
            input.value = "abcdefghijklmnopqrstuvwxyz";
            validate(passwordGroup);
            expect($passwordError.html()).to.equal("Password is Incorrect: Please enter no more than 12 characters");
        });
        it("isn't long password", function () {
            input.value = "Password123";
            validate(passwordGroup);
            expect($passwordError.html()).to.not.contain("12 characters");
        });

We add a checkPasswordLong reference to the password list,

        // "Password": [checkEmpty, checkFake, checkPasswordShort]
        "Password": [checkEmpty, checkFake, checkPasswordShort, checkPasswordLong]

and code to check for a long password uses the high text length regular expression.

    function checkPasswordLong(inputGroup) {
        const pswReghigh = /^([a-zA-Z0-9]{13,})$/;
        const value = getValue(inputGroup);
        if (pswReghigh.test(value)) {
            inputStatus.warning(inputGroup, getName(inputGroup) + " is Incorrect: Please enter no more than 12 characters");
            return false;
        }
        return true;
    }

Summary

We have used tests to recreate the validate function, improving the structure as we come across problems, while ensuring that those tests properly support the code.

The code as it stands today is found at v0.0.20 in releases

Next time we clean up some issues with the existing tests, that are no longer needed.

[Paul_Wilkins]

Finishing off the password retype tests and code is what we’re up to today. We will be doing the following:

• completing the validate() support for password retype
• get started at making validate() more configurable
• preventing trouble with unknown validation types

Adding support for password retype

When I turned on all of the tests after the previous post, I found that the change-password input still needs the retype to be dealt with.

Completing the validate password code for the change-password form is what next needs to be done. Here is the test failure tha’s reported to us: TypeError: Cannot read property 'every' of undefined at validateByTypes

This is a good time to think about making the validate code more modular, so that we can add different types of validation on a need-by-need basis.

We can add the password-retype code in the same way that we have been doing so currently, but we will update the validate function afterwards so that we can adds validators in more of a modular manner.

Test and code for Retyping the password

The tests for retyping the password are done one at a time, as per usual.

The test for using the validate function to check if the retype is empty, is as follows:

validate.test.js

    describe("retype password", function () {
        const retypeGroup = $("#changepw .form-group").has("[name='Password Retype']").get(0);
        const retypeInput = $(retypeGroup).find("input").get(0);
        const $retypeError = $(retypeGroup).find(".error");
        it("is empty", function () {
            retypeInput.value = "";
            validate(retypeGroup);
            expect($retypeError.html()).to.equal("Password Retype is empty");
        });
    });

As the validate function receives an HTML element reference for the form group, and not a jQuery object, I’ve ensured that the test has retypeGroup as an HTML element reference too.

The checkEmpty validation is already in place, we just need to add a Password Retype section to the validate function:

validate.js

        "Password": [checkEmpty, checkFake, checkPasswordShort, checkPasswordLong],
        "Password Retype": [checkEmpty]

and the test passes well.

Test and code for non-matching password

The test for a non-matching password means accessing the password input, as well as the retype input.

validate.test.js

    describe("retype password", function () {
        const passwordGroup = $("#changepw .form-group").has("[name='Password']").get(0);
        const passwordInput = $(passwordGroup).find("input").get(0);
        const retypeGroup = $("#changepw .form-group").has("[name='Password Retype']").get(0);
        const retypeInput = $(retypeGroup).find("input").get(0);
        const $retypeError = $(retypeGroup).find(".error");
        it("is empty", function () {
            //...
        });
        it("doesn't match", function () {
            passwordInput.value = "Password123";
            retypeInput.value = "Password234";
            validate(retypeGroup);
            expect($retypeError.html()).to.equal("Password Retype is Incorrect: Password doesn't match retyped password");
        });
    });

Checking if the password is different is just a matter of getting both password and retype fields, and checking that the values match.

validate.js

        "Password Retype": [checkEmpty, checkPasswordDifferent]
//...
    function checkPasswordDifferent(inputGroup) {
        const $form = $(inputGroup).closest("form");
        const $passwordInput = $form.find("[name=Password]");
        const $retypeInput = $(inputGroup).find("input");
        if ($passwordInput.val() !== $retypeInput.val()) {
            inputStatus.warning(inputGroup, getName(inputGroup) + " is Incorrect: Password doesn't match retyped password");
            return false;
        }
        return true;
    }

Finish off retype tests by checking for success

The previous test gave us the variables that we need, so the test is an easy matter of setting both input values the same and checking the error message.

    describe("retype password", function () {
        //...
        it("matches", function () {
            passwordInput.value = "Password123";
            retypeInput.value = "Password123";
            validate(retypeGroup);
            expect($retypeError.html()).to.equal("Password Retype is Ok: Your data has been entered correctly");
        });
    });

That test passes immediately and ends up acting as a verification test that the retype does show a suitable message when it is the same as the password.

Future planning

The validation function is now complete, for the login and change-password requirements. There is though also the registration form to consider.

The validate function has two potential futures ahead of it. Either the function can continue to grow and have a wide variety of validations built into it to support the registration form, or we can use a function argument to pass in other validations to the validate function that we want it to support.

I will go for the latter option, as that results in better flexibility for the validate function, allowing it to possibly be more easily used in other projects too.

Making validate() more configurable

We can move the retype checks out of the function, to help make the code more configurable and capable of responding to different circumstances.

Instead of having the retypePassword section in the validate function, we want to be able to pass in to the function the other validators that we are using.

A test is very good at helping to define those relationships.

    describe("uses custom validator to validate form field", function () {
        const inputGroup = $(".form-group").has("input").get(0);
        const validator = {
            "First Name": [function () {}]
        };
        validate(inputGroup, validator);
    });

Ensure that we hae a solid foundation

That test causes validate to break, with the following error: TypeError: Cannot read property 'every' of undefined

We really should ensure that the validate function gives a better message when a validator isn’t found.

In the validateByTypes function we can check if there are any types to check, and give an error message.

    function validateByTypes(inputGroup) {
        const name = getName(inputGroup);
        const types = validationTypes[name];
        if (!types) {
            throw new Error(name + " validation not yet supported");
        }
        return types.every(function (check) {
            return check(inputGroup);
        });
    }

That helps to give more information about the issue. I am now told by the test: Error: undefined validation not yet supported at validateByTypes letting me now know that the form field that I’m testing doesn’t have an input-check name.

Some parts of the form use check and others use input-check, both meaning that the form field is required.

There are a couple of ways to deal with that, and I’m pretty sure that I know how I want to solve that, but I’ll leave that for next time.

Leave things with a failing test?

When in the middle of developing code it can be very useful to end your day with a failing test. If you end with everything working, it takes longer to get started next time as you’re looking around trying to figure out what needs being worked on. When there is a failing test, that helps to give you immediate direction about what to work on next.

Summary

Today we completed the validate() support for password retype, and made a start at making it more configurable, finding that we should prevent trouble with unknown validation types.

As I try to avoid committing failing tests, the code as it was after finishing the retype tests and code is available as v0.0.21 in releases

Next time we finish adding configuration support to the validate() code.

[Paul_Wilkins]

Adding custom validators to the the validate() code, is what we’re up to today. We will be doing the following:

• develop a custom validator
• use validate.check() instead
• make validate functions public
• use a custom validator

Use an unknown validation to develop configuration

I want to remain quite firmly in the mindset of removing duplication. We know that the removal of duplication will result in more code using the validate() function. We don’t want a large set of rules to be in the validate function. We can instead update the validate code to accept custom validators.

The password-retype rules are quite specific. They shouldn’t be inside of the validate() function, and should instead be given as a custom configuration. To develop that custom configuration, we really can’t use the existing password-retype as the validate() function already knows about them.

That means, using something else such as First Name, a currently unknown type of field to the validate function, to develop passing custom validation rules.

Ignore required status on input fields

We left things off last time with a failing test, about an undefined validator. Error: undefined validation not yet supported at validateByTypes

The validate() function looks for elements with an input-check class, whereas other parts of a form use just check instead. Both of those indicate that the input field is required. Looking for either or both of those class names is not a suitable solution.

Why is using input-check or check a bad solution? The validate() function doesn’t care about whether an input field is required or not. Its only care about validating the input field that’s given to it. It shouldn’t have to care about whether the field is a required one or not.

Instead of looking for input-check or check, we can just look for an input field instead.

    function getName(inputGroup) {
        // return $(inputGroup).find(".input-check").attr("name");
        return $(inputGroup).find("input").attr("name");
    }
    function getValue(inputGroup) {
        // return $(inputGroup).find(".input-check").val().trim();
        return $(inputGroup).find("input").val().trim();
    }

and we are now given a more appropriate error message: Error: First Name validation not yet supported at validateByTypes

A firstname custom validator test

The custom validate test uses a spy to check if the validator function gets called by the validate function.

    describe("uses custom validator to validate form field", function () {
        const inputGroup = $(".form-group").has("input").get(0);
        it("can use a custom validator", function () {
            const validatorFn = function () {};
            const spy = chai.spy(validatorFn);
            const validator = {
                "First Name": [spy]
            };
            validate(firstnameGroup, validator);
            expect(spy).to.have.been.called();
        });
    });

We need the validate function to accept a validator object, and add it to the list of existing validators.

We can rename the validationTypes object to one called defaultValidators, and use Object.assign() to add together the default set of validators with the validators given to the validate function.

function validate(inputGroup, validators) {
    const defaultValidators = {
        "E-mail": [checkEmpty, checkFake, checkEmailReg],
        "Password": [checkEmpty, checkFake, checkPasswordShort, checkPasswordLong],
        "Password Retype": [checkEmpty, checkPasswordDifferent]
    };
    const validationTypes = Object.assign(defaultValidators, validators);

We can use two more tests to confirm that the custom validation properly occurs.

        describe("custom first-name validator", function () {
            const $firstnameInput = $(".form-group").find("input");
            const $firstnameError = $(".form-group").find(".error");
            function isLessThanThree(inputGroup) {
                const input = $(inputGroup).find("input");
                if (input.val().length < 3) {
                    inputStatus.warning(inputGroup, "Shouldn't be less than three characters");
                    return false;
                }
                return true;
            }
            const customValidator = {
                "First Name": [isLessThanThree]
            }
            it("checks if name is less than 3 characters", function () {
                $firstnameInput.val("Ma");
                $firstnameError.html();
                validate(firstnameGroup, customValidator);
                expect($firstnameError.html()).to.contain("less than three");
            });
            it("can use a custom validator", function () {
                $firstnameInput.val("Mat");
                $firstnameError.html();
                validate(firstnameGroup, customValidator);
                expect($firstnameError.html()).to.contain("Ok");
            });
        });

Those two tests help to confirm that the custom validator gives suitable fail and pass messages.

Move retype-password tests out of validate()

We can now move the retype-password tests. First we remove them out of the validate() function:

validate.js

    const defaultValidators = {
        "E-mail": [checkEmpty, checkFake, checkEmailReg],
        "Password": [checkEmpty, checkFake, checkPasswordShort, checkPasswordLong],
        // "Password Retype": [checkEmpty, checkPasswordDifferent]
    };

That causes some tests to fail. We can then add the reset-password validator to the change-password code:

change-password.js

    function checkPasswordDifferent(inputGroup) {
        const $form = $(inputGroup).closest("form");
        const $passwordInput = $form.find("[name=Password]");
        const $retypeInput = $(inputGroup).find("input");
        if ($passwordInput.val() !== $retypeInput.val()) {
            inputStatus.warning(inputGroup, validate.fn.getName(inputGroup) + " is Incorrect: Password doesn't match retyped password");
            return false;
        }
        return true;
    }
    const retypeValidator = {
        "Password Retype": [checkEmpty, checkPasswordDifferent]
    };

We can’t copy out the checkEmpty function though as that would be duplication, so we need to access the checkEmpty function in the validate() code.

To achieve that we want the validate function to auto-exec, adding common validation functions to a publicly accessible object. That also means using the publicly accessible object to check if something is valid.

The order of changes are:

1. change tests to use validate.check()
2. use an IIFE for the validate.check()
3. add checkEmpty and other functions to validate.fn{}

Change validate tests to use validate.check()

Updating the validate tests so that they use validate.check() is a good first step here:

        it("can use a custom validator", function () {
            //...
            // validate(firstnameGroup, customValidator);
            validate.check(firstnameGroup, customValidator);
            expect(spy).to.have.been.called();
        });
//...
        it("can use a custom validator", function () {
            //...
            // validate(firstnameGroup, customValidator);
            validate.check(firstnameGroup, customValidator);
            expect(spy).to.have.been.called();
        });
//...
            it("checks if name is less than 3 characters", function () {
                //...
                // validate(firstnameGroup, customValidator);
                validate.check(firstnameGroup, customValidator);
                expect($firstnameError.html()).to.contain("less than three");
            });
            it("can use a custom validator", function () {
                //...
                // validate(firstnameGroup, customValidator);
                validate.check(firstnameGroup, customValidator);
                expect($firstnameError.html()).to.contain("Ok");
            });

The tests show an error of TypeError: validate.check is not a function so we can use those to help get us to a working solution.

We need validate.check to be found. We can rename the validate function to check, and add the module pattern at the end to return that check function in the returned object.

// function validate(inputGroup, validators) {
function check(inputGroup, validators) {
    //...
}
const validate = (function () {
    return {
        check
    };
}());

Other tests now say TypeError: validate is not a function so we can go to each of those problems and rename validate to be validate.check instead.

For example, somewhere in validate.test.js:

        it("is empty", function () {
            input.value = "";
            // validate(emailGroup);
            validate.check(emailGroup);
            expect($emailError.html()).to.equal("E-mail is empty");
        });

We then have some references to validate in change-password.js that need updating:

    function passwordInputHandler() {
        // validate(this);
        validate.check(this);
    }
    //...
    // $("#changepw .form-group").on("focusin focusout input", validate);
    $("#changepw .form-group").on("focusin focusout input", validate.check);

and the tests pass once more.

Reorganise validate code

We can now move the check function inside of the validate code:

// function check(inputGroup, validators) {
//     ...
// }
const validate = (function () {
    function check(inputGroup, validators) {
        // ...
    }
}());

Make validation functions public

The main goal here was to make the validation functions available, which we can do now.

We can extract out the functions from inside of that check function, so that they are above the check function instead.

const validate = (function () {
    function check(inputGroup, validators) {
        // ...
    }
    //...
    function showValid(inputGroup) {
        //...
    }
    function check(inputGroup, validators) {
        const validationTypes = Object.assign(defaultValidators, validators);
        function validateByTypes(inputGroup) {
            //...
        }
        const isValid = validateByTypes(inputGroup);
        if (isValid) {
            showValid(inputGroup);
        }
    }
    return {
        check
    };
}());

and access the frequently used validator functions from the returned object, making them available in an fn reference:

    return {
        check,
        fn: {
            getName,
            getValue,
            checkEmpty,
            checkFake
        }
    };

Use a custom validator

We can now update change-password so that it uses custom validatePassword code instead.

const changePassword = (function() {
    function checkPasswordDifferent(inputGroup) {
        const $form = $(inputGroup).closest("form");
        const $passwordInput = $form.find("[name=Password]");
        const $retypeInput = $(inputGroup).find("input");
        if ($passwordInput.val() !== $retypeInput.val()) {
            // inputStatus.warning(inputGroup, getName(inputGroup) + " is Incorrect: Password doesn't match retyped password");
            inputStatus.warning(inputGroup, validate.fn.getName(inputGroup) + " is Incorrect: Password doesn't match retyped password");
            return false;
        }
        return true;
    }
    const retypeValidator = {
        "Password Retype": [validate.fn.checkEmpty, checkPasswordDifferent]
    };
    function passwordInputHandler() {
        validate.check(this, retypeValidator);
    }

Remove unneeded code and tests

We can now remove from the validate code the password retype code:

    const defaultValidators = {
        "E-mail": [checkEmpty, checkFake, checkEmailReg],
        "Password": [checkEmpty, checkFake, checkPasswordShort, checkPasswordLong],
        // "Password Retype": [validate.fn.checkEmpty, checkPasswordDifferent]
    };
    //...
    // function checkPasswordDifferent(inputGroup) {
        //...
    // }

The validate tests for password retype now fail. They aren’t needed anymore as we are now using custom password retype validations from the change-password code. The validate tests for retype password can now be removed.

    // describe("retype password", function () {
        //...
    // });

and that gives us good direction about what to work on next.

Summary

Today we developed a custom validator that uses validate.check() instead, and made the validate functions public so that we can create a custom validator.

The code as it stands today is found at v0.0.22 in releases

Thanks to the benefits gained from the validate() code, the other tests don’t need as detailed as they were before.

Next time, we look through our tests and figure out how to tell when some of them aren’t needed anymore.

[Paul_Wilkins]

Using the login and change-password as an example, when duplication is removed from both of them by moving code to a separate set of code such as the validate code, detailed tests are still required for validate, but don’t need to be as detailed for the login and change-password code.

Today we will be:

• adding inputStatus tests
• removing duplication from other tests

Investigating the other tests

Before that though, the existing tests in login-input-email.test.js and other tests, were put in place before the details were all contained within the inputStatus set of code. As the login-input-email.js code is no longer responsible for those details, we need to provide tests for the inputStatus code and remove them where they’re no longer needed from the login input tests.

Adding inputStatus tests

The inputStatus code needs its own set of tests so that the things that it does can be neatly controlled from the one area. That area is the inputStatus.test.js set of tests.

inputStatus.test.js

describe("input status", function () {
    inputGroup = $(".form-group").has(".error").get(0);
    $error = $(inputGroup).find(".error");
    $feedback = $(inputGroup).find(".feedback");
    $required = $(inputGroup).find(".starrq");
    it("setNone removes ok", function () {
        $error.addClass("ok");
        inputStatus.setNone($error);
        expect($error.attr("class")).to.not.contain("ok");
    });
    it("setNone removes warning", function () {
        $error.addClass("warning");
        inputStatus.setNone($error);
        expect($error.attr("class")).to.not.contain("warning");
    });
    it("setOk", function () {
        $error.removeClass("ok");
        inputStatus.setOk($error);
        expect($error.attr("class")).to.contain("ok");
    });
    it("setWarning", function () {
        $error.removeClass("warning");
        inputStatus.setWarning($error);
        expect($error.attr("class")).to.contain("warning");
    });
    it("errorOk shows message", function () {
        $error.html("");
        inputStatus.errorOk(inputGroup, "Test message");
        expect($error.html()).to.equal("Test message");
    });
    it("errorOk sets color", function () {
        const cssGreen = "rgb(0, 128, 0)";
        $error.css("color", "red");
        inputStatus.errorOk(inputGroup, "Test message");
        expect($error.css("color")).to.equal(cssGreen);
    });
    it("errorOk sets ok", function () {
        $error.removeClass("ok");
        inputStatus.errorOk(inputGroup, "Test message");
        expect($error.attr("class")).to.contain("ok");
    });
    it("errorWarning shows message", function () {
        $error.html("");
        inputStatus.errorWarning(inputGroup, "Test message");
        expect($error.html()).to.equal("Test message");
    });
    it("errorWarning sets color", function () {
        const cssRed = "rgb(255, 0, 0)";
        $error.css("color", "green");
        inputStatus.errorWarning(inputGroup, "Test message");
        expect($error.css("color")).to.equal(cssRed);
    });
    it("errorWarning sets warning", function () {
        $error.removeClass("warning");
        inputStatus.errorWarning(inputGroup, "Test message");
        expect($error.attr("class")).to.contain("warning");
    });
    describe("feedback", function () {
        it("feedbackNone removes glyphicon", function () {
            $feedback.addClass("glyphicon");
            inputStatus.feedbackNone(inputGroup);
            expect($feedback.attr("class")).to.not.contain("glyphicon");
        });
        it("feedbackNone removes glyphicon-remove", function () {
            $feedback.addClass("glyphicon-remove");
            inputStatus.feedbackNone(inputGroup);
            expect($feedback.attr("class")).to.not.contain("glyphicon-remove");
        });
        it("feedbackNone removes glyphicon-ok", function () {
            $feedback.addClass("glyphicon-ok");
            inputStatus.feedbackNone(inputGroup);
            expect($feedback.attr("class")).to.not.contain("glyphicon-ok");
        });
        it("feedbackNone removes ok", function () {
            $feedback.addClass("ok");
            inputStatus.feedbackNone(inputGroup);
            expect($feedback.attr("class")).to.not.contain("ok");
        });
        it("feedbackNone removes warning", function () {
            $feedback.addClass("warning");
            inputStatus.feedbackNone(inputGroup);
            expect($feedback.attr("class")).to.not.contain("warning");
        });
        it("feedbackOk adds glyphicon", function () {
            $feedback.removeClass("glyphicon");
            inputStatus.feedbackOk(inputGroup);
            expect($feedback.attr("class")).to.contain("glyphicon");
        });
        it("feedbackOk removes glyphicon-remove", function () {
            $feedback.addClass("glyphicon-remove");
            inputStatus.feedbackOk(inputGroup);
            expect($feedback.attr("class")).to.not.contain("glyphicon-remove");
        });
        it("feedbackOk adds glyphicon-ok", function () {
            $feedback.removeClass("glyphicon-ok");
            inputStatus.feedbackOk(inputGroup);
            expect($feedback.attr("class")).to.contain("glyphicon-ok");
        });
        it("feedbackOk adds ok", function () {
            $feedback.removeClass("ok");
            inputStatus.feedbackOk(inputGroup);
            expect($feedback.attr("class")).to.contain("ok");
        });
        it("feedbackOk removes warning", function () {
            $feedback.addClass("warning");
            inputStatus.feedbackOk(inputGroup);
            expect($feedback.attr("class")).to.not.contain("warning");
        });
        it("feedbackWarning adds glyphicon", function () {
            $feedback.removeClass("glyphicon");
            inputStatus.feedbackWarning(inputGroup);
            expect($feedback.attr("class")).to.contain("glyphicon");
        });
        it("feedbackWarning adds glyphicon-remove", function () {
            $feedback.removeClass("glyphicon-remove");
            inputStatus.feedbackWarning(inputGroup);
            expect($feedback.attr("class")).to.contain("glyphicon-remove");
        });
        it("feedbackWarning removes glyphicon-ok", function () {
            $feedback.addClass("glyphicon-ok");
            inputStatus.feedbackWarning(inputGroup);
            expect($feedback.attr("class")).to.not.contain("glyphicon-ok");
        });
        it("feedbackWarning removes ok", function () {
            $feedback.addClass("ok");
            inputStatus.feedbackWarning(inputGroup);
            expect($feedback.attr("class")).to.not.contain("ok");
        });
        it("feedbackWarning adds warning", function () {
            $feedback.removeClass("warning");
            inputStatus.feedbackWarning(inputGroup);
            expect($feedback.attr("class")).to.contain("warning");
        });
    });
    describe("required", function () {
        it("requiredOk", function () {
            $required.removeClass("ok");
            inputStatus.requiredOk(inputGroup);
            expect($required.attr("class")).to.contain("ok");
        });
        it("requiredWarning", function () {
            $required.removeClass("warning");
            inputStatus.requiredWarning(inputGroup);
            expect($required.attr("class")).to.contain("warning");
        });
    });
    it("ok shows message", function () {
        $error.html();
        inputStatus.ok(inputGroup, "Test message");
        expect($error.html()).to.equal("Test message");
    });
    it("ok sets feedback", function () {
        $feedback.removeClass("ok");
        inputStatus.ok(inputGroup, "Test message");
        expect($feedback.attr("class")).to.contain("ok");
    });
    it("ok sets required", function () {
        $required.removeClass("ok");
        inputStatus.ok(inputGroup, "Test message");
        expect($required.attr("class")).to.contain("ok");
    });
    it("warning shows message", function () {
        $error.html();
        inputStatus.warning(inputGroup, "Test message");
        expect($error.html()).to.equal("Test message");
    });
    it("warning sets feedback", function () {
        $feedback.removeClass("warning");
        inputStatus.warning(inputGroup, "Test message");
        expect($feedback.attr("class")).to.contain("warning");
    });
    it("warning sets required", function () {
        $required.removeClass("warning");
        inputStatus.warning(inputGroup, "Test message");
        expect($required.attr("class")).to.contain("warning");
    });
    describe("resetForm", function () {
        $form = $("form").eq(1);
        $formGroups = $form.find(".form-group").has(".input-check");
        $group1 = $formGroups.eq(0);
        $group2 = $formGroups.eq(1);
        $name1 = $group1.find(".input-check").attr("name");
        $name2 = $group2.find(".input-check").attr("name");
        $error1 = $group1.find(".error");
        $error2 = $group2.find(".error");
        $feedback1 = $group1.find(".feedback");
        $feedback2 = $group2.find(".feedback");
        it("resets one message", function () {
            $error1.find(".error").html("");
            inputStatus.resetForm($form);
            expect($error1.html()).to.equal("Your " + $name1);
        });
        it("resets another message", function () {
            $error2.find(".error").html("");
            inputStatus.resetForm($form);
            expect($error2.html()).to.equal("Your " + $name2);
        });
        it("resets one feedback", function () {
            $feedback1.addClass("warning");
            inputStatus.resetForm($form);
            expect($feedback1.attr("class")).to.not.contain("warning");
        });
        it("resets another feedback", function () {
            $feedback2.addClass("warning");
            inputStatus.resetForm($form);
            expect($feedback2.attr("class")).to.not.contain("warning");
        });
    });
});

Now that the inputStatus tests are in their proper place, we can remove almost all of the other error and feedback and required tests from the other code.

Removing unneeded tests

Many of the inputStatus tests remove the need for so much detail in the other tests. Because inputStatus is being used in the code to update the fields, we only need to check that one suitable change is being made, not all of them. Because the error text is the most varied, that is about all that we need to test. The other tests can be removed.

I’ll work through each of the test files alphabetically to help me avoid missing anything, examining each section as we go.

Remove unneeded tests from change-password tests

In the changepassword-input-email.test.js file we can remove all of the tests that check the class names for error, and feedback, as those are already all tested already in the inputStatus code.

    describe("email is fake", function () {
        beforeEach(function () {
            $emailInput.val("aaabbb@example.com");
        })
        it("shows a message", function () {
            $emailError.html("");
            passwordInputHandler();
            expect($emailError.html()).to.equal("E-mail is Fake text: Please remove repetition");
        });
        // it("adds warning to error", function () {
            //...
        // });
        // it("removes ok from error", function () {
            //...
        // });
        // it("removes glyphicon-ok from feedback", function () {
            //...
        // });
        // it("adds glyphicon to feedback", function () {
            //...
        // });
        // it("adds glyphicon-remove to feedback", function () {
            //...
        // });
        // it("removes ok from feedback", function () {
            //...
        // });
        // it("adds warning to feedback", function () {
            //...
        // });
    });

We can do the same with all other changepassword-input-email tests too.

After doing that, can we confirm that the limited number of tests still catch all possible problems? We sure can.

All of the code relating to the change-password input is as follows:

    function passwordInputHandler() {
        validate.check(this, retypeValidator);
    }
    //...
    $("#changepw .form-group").on("focusin focusout input", validate.check);

Changing any of those lines causes the tests to suitably fail, so all is good.

Remove other unneeded tests

We can now work through the other test files doing similar removal of unneeded tests that inputStatus already tests.

[Lots of test files have things deleted from them here]

It’s tempting to feel bad about all of this removal, but they’ve done their job. They ensured that the duplication removal didn’t result in any change to the behaviour of the code, and aren’t needed anymore as what they test is a duplicate of what is alrady tested in the inputStatus code.

Another benefit is that we go from nearly 500 tests taking several seconds to test, down to nearly 100 tests that run in about a second. That’s much better.

Summary

Today we added inputStatus tests, and removed unneeded tests from other tests that already duplicate the inputStatus tests.

The code as it stands today is found at v0.0.23 in releases

Next time we can carry on using jsInspect to find duplication for us to remove.

[Paul_Wilkins]

Removing duplication from postal and zip code inputs is what we’re up to today. We will be doing the following:

• removing some useless comments
• adding tests for postal code and zip code
• using inputStatus and validate to remove duplication
• make a createValidator function to remove further duplication

Remove duplication from postal address and zip code

There are 6 sets of duplication remaining that jsInspect informs us about. The next set of duplication is the postal and zip code inputs.

./registration3\registration.js:513,519
    $("#postalRequired").removeClass("ok").addClass("warning");
} else {
    $(this).next().find(".error").html(name + " is Ok : Your data has been entered correctly");
    $(this).next().find(".error").addClass('ok').removeClass('warning');
    $(this).next().find(".feedback").removeClass("glyphicon glyphicon-remove").addClass("glyphicon glyphicon-ok").removeClass("warning").addClass("ok");
    $("#postalRequired").removeClass("warning").addClass("ok");
}

./registration3\registration.js:536,542
    $("#zipRequired").removeClass("ok").addClass("warning");
} else {
    $(this).next().find(".error").html(name + " is Ok : Your data has been entered correctly");
    $(this).next().find(".error").addClass('ok').removeClass('warning');
    $(this).next().find(".feedback").removeClass("glyphicon glyphicon-remove").addClass("glyphicon glyphicon-ok").removeClass("warning").addClass("ok");
    $("#zipRequired").removeClass("warning").addClass("ok");
}

Even though it’s pretty clear that inputStatus will be our solution, proprieties must be observed so tests for the existing code are put in place, before making an changes. We will test for the different error messages that are given. As the presentation aspects are going to be immediately replaced with the inputStatus code, there’s no point adding tests for the presentation aspects, as inputStatus already has those in place for us.

Remove useless comments

While looking through that code I cleaned up some useless comments by removing them. For example:

    /* first name */
    if (name === "First Name") {

Such comments are not needed at all. I remove useless comments like that on sight, and they have been removed from the registration code.

Postal address tests

The postal address tests are easy to put in place for all three types of error messages.

tests/tests.html

<script src="registration-input-email.test.js"></script>
<script src="registration-input-postaladdress.test.js"></script>

tests/registration-input-postaladdress.test.js

describe("registration input postaladdress", function () {
    /*
       Structure
       - .form-group
         - .starrq
         - .input-group
           - input
         - .inputstatus
           - .error
           - .feedback
    */
    function callRegistrationInputHandler(thisArg) {
        const registrationInputHandler = registration.eventHandler.registrationInput;
        registrationInputHandler.call(thisArg);
    }
    const $postalGroup = $(".form-group").has("[name='Postal Address']");
    const $postalInputGroup = $postalGroup.find(".input-group");
    const $postalInput = $postalGroup.find("textarea");
    const $postalError = $postalGroup.find(".error");
    after(function () {
        $("#registration").trigger("reset");
    });
    it("when address is empty", function () {
        $postalInput.val("");
        $postalError.html("");
        $postalError.removeClass("warning");
        callRegistrationInputHandler($postalInputGroup);
        expect($postalError.html()).to.equal("Postal Address is EMPTY: Please enter data into this input");
        expect($postalError.attr("class")).to.contain("warning");
    });
    it("when address is not valid", function () {
        $postalInput.val("not valid");
        $postalError.html("");
        $postalError.removeClass("warning");
        callRegistrationInputHandler($postalInputGroup);
        expect($postalError.html()).to.equal("Postal Address is Incorrect: Please enter Address correctly");
        expect($postalError.attr("class")).to.contain("warning");
    });
    it("when address is valid", function () {
        $postalInput.val("123 Test Lane");
        $postalError.html("");
        $postalError.removeClass("ok");
        callRegistrationInputHandler($postalInputGroup);
        expect($postalError.html()).to.equal("Postal Address is Ok: Your data has been entered correctly");
        expect($postalError.attr("class")).to.contain("ok");
    });
});

I’ve included a few simple class checks for warning or ok, to help check for obvious issues when converting the code.

The postal address tests all pass, and we can start updating the code.

Use inputStatus and reorganise the code

First we add a $formGroup variable, and use inputStatus for each of the error warnings and messages:

        if (name === "Postal Address") {
            const $formGroup = $(this).closest(".form-group");
            if (value.length > 0) {
                var AddressReg = /^\d+\s[A-z]+\s[A-z]+/g;
                if (!AddressReg.test(value)) {
                    inputStatus.warning($formGroup, name + " is Incorrect: Please enter Address correctly");
                } else {
                    inputStatus.ok($formGroup, name + " is Ok: Your data has been entered correctly");
                }
            } else {
                inputStatus.warning($formGroup, name + " is EMPTY: Please enter data into this input");
            }
        }

Then, as the tests still pass, we can reorganise the code. The usual order is empty text first, and successful results last.

        if (name === "Postal Address") {
            const $formGroup = $(this).closest(".form-group");
            const AddressReg = /^\d+\s[A-z]+\s[A-z]+/g;
            if (value === "") {
                inputStatus.warning($formGroup, name + " is EMPTY: Please enter data into this input");
            } else if (!AddressReg.test(value)) {
                inputStatus.warning($formGroup, name + " is Incorrect: Please enter Address correctly");
            } else {
                inputStatus.ok($formGroup, name + " is Ok: Your data has been entered correctly");
            }
        }

And we can now move that code into a custom validator.

        if (name === "Postal Address") {
            const $formGroup = $(this).closest(".form-group");
            function checkPostalAddress(inputGroup) {
                const $postalInput = $(inputGroup).find("textarea");
                const value = validate.fn.getValue(inputGroup);
                const addressReg = /^\d+\s[A-z]+\s[A-z]+/g;
                if (!addressReg.test(value)) {
                    inputStatus.warning(inputGroup, validate.fn.getName(inputGroup) + " is Incorrect: Please enter Address correctly");
                    return false;
                }
                return true;
            }
            validate.check($formGroup, {
                "Postal Address": [validate.fn.checkEmpty, checkPostalAddress]
            });
        }

The tests fail briefly, because the getName function looks for an input field, but a textarea is expected.

As textarea is a valid type of input field we can add that to the getName and getValue methods in the validate function:

validate.js

    function getName(inputGroup) {
        // return $(inputGroup).find("input").attr("name");
        return $(inputGroup).find("input, textarea").attr("name");
    }
    function getValue(inputGroup) {
        // return $(inputGroup).find("input").val().trim();
        return $(inputGroup).find("input, textarea").val().trim();
    }

And the last remaining failing test is about the message when a field is empty.

Using a consistent error message

One of the empty messages is from the validate code that says Postal Address is empty and the other is more explicit Postal Address is EMPTY: Please enter data into this input

I think that it makes sense to use the more explicit version. Staying consistent with the other error messages though, I’ll just use a capital first letter on Empty.

We update the validate checkEmpty message:

    function checkEmpty(inputGroup) {
        if (getValue(inputGroup) === "") {
            // inputStatus.warning(inputGroup, getName(inputGroup) + " is empty");
            inputStatus.warning(inputGroup, getName(inputGroup) + " is Empty: Please enter data into this input");
            return false;
        }
        return true;
    }

And we are informed of other tests that want updating in regard to the empty message, for example in the login-input-password.test.js file:

    it("password is empty", function () {
        $passwordInput.val("");
        $passwordError.html("");
        loginInputHandler($passwordGroup);
        // expect($passwordError.html()).to.equal("Password is empty");
        expect($passwordError.html()).to.equal("Password is Empty: Please enter data into this input");
    });

and the tests all pass once more.

Zip code tests

The zip code tests are similar to the postal address tests:

describe("registration input zipcode", function () {
    /*
       Structure
       - .form-group
         - .starrq
         - .input-group
           - input
         - .inputstatus
           - .error
           - .feedback
    */
    function callRegistrationInputHandler(thisArg) {
        const registrationInputHandler = registration.eventHandler.registrationInput;
        registrationInputHandler.call(thisArg);
    }
    const $zipcodeGroup = $(".form-group").has("[name='zip code']");
    const $zipcodeInputGroup = $zipcodeGroup.find(".input-group");
    const $zipcodeInput = $zipcodeGroup.find("input");
    const $zipcodeError = $zipcodeGroup.find(".error");
    after(function () {
        $("#registration").trigger("reset");
    });
    it("when zipcode is empty", function () {
        $zipcodeInput.val("");
        $zipcodeError.html("");
        $zipcodeError.removeClass("warning");
        callRegistrationInputHandler($zipcodeInputGroup);
        expect($zipcodeError.html()).to.equal("zip code is EMPTY: Please enter data into this input");
        expect($zipcodeError.attr("class")).to.contain("warning");
    });
    it("when zipcode is not valid", function () {
        $zipcodeInput.val("notvalid");
        $zipcodeError.html("");
        $zipcodeError.removeClass("warning");
        callRegistrationInputHandler($zipcodeInputGroup);
        expect($zipcodeError.html()).to.equal("zip code is Incorrect: Please enter Post-code correctly");
        expect($zipcodeError.attr("class")).to.contain("warning");
    });
    it("when zipcode is valid", function () {
        $zipcodeInput.val("PI1 2ZA");
        $zipcodeError.html("");
        $zipcodeError.removeClass("ok");
        callRegistrationInputHandler($zipcodeInputGroup);
        expect($zipcodeError.html()).to.equal("zip code is Ok: Your data has been entered correctly");
        expect($zipcodeError.attr("class")).to.contain("ok");
    });
});

Use inputStatus and validate for postcode

We can update the code in the usual way using inputStatus,

        if (name === "zip code") {
            const $formGroup = $(this).closest(".form-group");
            const PostcodeReg = /^[a-zA-Z]{1,2}([0-9]{1,2}|[0-9][a-zA-Z])\s*[0-9][a-zA-Z]{2}$/;
            if (value.length > 0) {
                if (!PostcodeReg.test(value)) {
                    inputStatus.warning($formGroup, name + " is Incorrect: Please enter Post-code correctly");
                } else {
                    inputStatus.ok($formGroup, name + " is Ok: Your data has been entered correctly");
                }
            } else {
                inputStatus.warning($formGroup, name + " is EMPTY: Please enter data into this input");
            }
        }

The tests all still pass, so we can reorganise the code with empty first, and successful last.

        if (name === "zip code") {
            const $formGroup = $(this).closest(".form-group");
            const PostcodeReg = /^[a-zA-Z]{1,2}([0-9]{1,2}|[0-9][a-zA-Z])\s*[0-9][a-zA-Z]{2}$/;
            if (value === "") {
                inputStatus.warning($formGroup, name + " is EMPTY: Please enter data into this input");
            } else if (!PostcodeReg.test(value)) {
                inputStatus.warning($formGroup, name + " is Incorrect: Please enter Post-code correctly");
            } else {
                inputStatus.ok($formGroup, name + " is Ok: Your data has been entered correctly");
            }
        }

And we can move most of that code into a checkPostcode function.

        if (name === "zip code") {
            const $formGroup = $(this).closest(".form-group");
            function checkPostcode(inputGroup) {
                const $postcodeInput = $(inputGroup).find("input");
                const value = validate.fn.getValue(inputGroup);
                const postcodeReg = /^[a-zA-Z]{1,2}([0-9]{1,2}|[0-9][a-zA-Z])\s*[0-9][a-zA-Z]{2}$/;
                if (!postcodeReg.test(value)) {
                    inputStatus.warning(inputGroup, validate.fn.getName(inputGroup) + " is Incorrect: Please enter Post-code correctly");
                    return false;
                }
                return true;
            }
            validate.check($formGroup, {
                "zip code": [validate.fn.checkEmpty, checkPostcode]
            });
        }

There is still a mental conflict over zipcode and postcode. I suspect that you intend to use postcode throughout, but I won’t change the field name from zipcode yet, until it’s confirmed that it won’t cause problems with things on the server side when the form is submitted.

Add a createValidator function

We are now seeing quite some duplication between the checkPostal and checkPostcode functions. There are only two differences between them. The the regular expression and the error message.

What can help is to use a factory function, to create the check function.

I want to pass a simple validation function, and an error message, and to receive a check function that can be used with validate.check. That is a new feature, so a test is required to define what we want to achieve.

validate.test.js

        it("createValidator finds invalid value", function () {
            $firstnameInput.val("Ma");
            const checkAtLeastThree = validate.createValidator(function (input) {
                return input.value.length >= 3;
            }, "Should be three or more characters");
            validate.check(firstnameGroup, {
                "First Name": [checkAtLeastThree]
            });
            expect($firstnameError.html()).to.equal("First Name is Incorrect: Should be three or more characters");
        });

I was tossing up as to whether to use the input field or the value for checking things, and the input seems the better choice as it gives more flexibility.

We can now add a createCheck function to the validate code, that takes the rule, checks it, and shows the warning message.

validate.js

    function createValidator(rule, errorMessage) {
        return function check(inputGroup) {
            const input = $(inputGroup).find("input, textarea").get(0);
            if (!rule(input)) {
                inputStatus.warning(inputGroup, validate.fn.getName(inputGroup) + " is Incorrect: " + errorMessage);
                return false;
            }
            return true;
        }
    }
    //...
    return {
        createValidator,
        check,
        //...
    };

That createValidator rule helps us to remove a lot of duplication for the postal address validation:

            // function checkPostalAddress(inputGroup) {
            //     const value = validate.fn.getValue(inputGroup);
            //     const addressReg = /^\d+\s[A-z]+\s[A-z]+/g;
            //     if (!addressReg.test(value)) {
            //         inputStatus.warning(inputGroup, validate.fn.getName(inputGroup) + " is Incorrect: Please enter Address correctly");
            //         return false;
            //     }
            //     return true;
            // }
            const addressRule = function (input) {
                const addressReg = /^\d+\s[A-z]+\s[A-z]+/g;
                return addressReg.test(input.value);
            };
            validate.check($formGroup, {
                "Postal Address": [
                    validate.fn.checkEmpty,
                    validate.createValidator(addressRule, "Please enter Address correctly")
                ]
            });

and for the postcode validation:

            // function checkPostcode(inputGroup) {
            //     const $postcodeInput = $(inputGroup).find("input");
            //     const value = validate.fn.getValue(inputGroup);
            //     const postcodeReg = /^[a-zA-Z]{1,2}([0-9]{1,2}|[0-9][a-zA-Z])\s*[0-9][a-zA-Z]{2}$/;
            //     if (!postcodeReg.test(value)) {
            //         inputStatus.warning(inputGroup, validate.fn.getName(inputGroup) + " is Incorrect: Please enter Post-code correctly");
            //         return false;
            //     }
            //     return true;
            // }
            const postcodeRule = function (input) {
                const postcodeReg = /^[a-zA-Z]{1,2}([0-9]{1,2}|[0-9][a-zA-Z])\s*[0-9][a-zA-Z]{2}$/;
                return postcodeReg.test(input.value);
            };
            validate.check($formGroup, {
                "zip code": [
                    validate.fn.checkEmpty,
                    validate.createValidator(postcodeRule, "Please enter Post-code correctly")
                ]
            });

There’s still a bit of duplication there, but it’s now much less reduced.

Use that createValidator function with other code

Now that we have the createValidator function, we can use that with other code in the change-password sections.

    // function checkPasswordDifferent(inputGroup) {
    //     const $form = $(inputGroup).closest("form");
    //     const $passwordInput = $form.find("[name=Password]");
    //     const $retypeInput = $(inputGroup).find("input");
    //     if ($passwordInput.val() !== $retypeInput.val()) {
    //         inputStatus.warning(inputGroup, validate.fn.getName(inputGroup) + " is Incorrect: Password doesn't match retyped password");
    //         return false;
    //     }
    //     return true;
    // }
    function passwordInputHandler() {
        const passwordsMatchRule = function (input) {
            const form = input.form;
            const passwordInput = form.elements["Password"];
            return passwordInput.value === input.value;
        }
        validate.check(this, {
            "Password Retype": [
                validate.fn.checkEmpty,
                validate.createValidator(passwordsMatchRule, "Password doesn't match retyped password")
            ]
        });
    }

Summary

Today we removed some useless comments, added tests for postal code and zip code, used inputStatus and validate to remove duplication, and made a createValidator function to remove even more duplication.

The code as it stands today is found at v0.0.24 in releases

Next time we use validate.check() to remove even more duplication from this area of code.

[Paul_Wilkins]

Removing duplication from firstname and lastname validation code is what we’re up to today. We will be doing the following:

• add tests for firstname and lastname validations
• simplify the validation structure
• create validation rule functions
• use those rules with validate.check

Investigating the next set of duplication

The next set of duplication is between the firstname and lastname sets of code.

./registration3\registration.js:377,383
    $("#fnameRequired").removeClass("ok").addClass("warning");
} else {
    $(this).next().find(".error").html(name + " is Incorrect: Please enter upper case and lower case only");
    $(this).next().find(".error").addClass('warning').removeClass('ok');
    $(this).next().find(".feedback").removeClass("glyphicon glyphicon-ok").addClass("glyphicon glyphicon-remove").removeClass("ok").addClass("warning");
    $("#fnameRequired").removeClass("ok").addClass("warning");
}

./registration3\registration.js:409,415
    $("#lnameRequired").removeClass("ok").addClass("warning");
} else {
    $(this).next().find(".error").html(name + " is Incorrect: Please enter upper case and lower case only");
    $(this).next().find(".error").addClass('warning').removeClass('ok');
    $(this).next().find(".feedback").removeClass("glyphicon glyphicon-ok").addClass("glyphicon glyphicon-remove").removeClass("ok").addClass("warning");
    $("#lnameRequired").removeClass("ok").addClass("warning");
}

Adding tests for first name

We already have some basic firstname test code for fake text, but we need to expand it to account for all types of firstname error messages.

registration-input-firstname.test.js

describe("registration-input first name", function () {
    /*
       Structure
       - .form-group
         - .starrq
         - .input-group
           - input
         - .inputstatus
           - .error
           - .feedback
    */
    function callRegistrationInputHandler(thisArg) {
        const registrationInputHandler = registration.eventHandler.registrationInput;
        registrationInputHandler.call(thisArg);
    }
    const $firstnameGroup = $(".form-group").has("[name='First Name']");
    const $firstnameInputGroup = $firstnameGroup.find(".input-group");
    const $firstnameInput = $firstnameGroup.find("input");
    const $firstnameError = $firstnameGroup.find(".error");
    after(function () {
        $("#registration").trigger("reset");
    });
    it("isn't empty", function () {
        $firstnameInput.val("");
        $firstnameError.html("");
        callRegistrationInputHandler($firstnameInputGroup);
        expect($firstnameError.html()).to.equal("First Name is Empty: Please enter data into this input");
    });
    it("isn't fake", function () {
        $firstnameInput.val("abbbc");
        $firstnameError.html("");
        callRegistrationInputHandler($firstnameInputGroup);
        expect($firstnameError.html()).to.equal("First Name is Fake text: Please remove repetition");
    });
    it("isn't too long", function () {
        $firstnameInput.val("Too much text in the input field");
        $firstnameError.html("");
        callRegistrationInputHandler($firstnameInputGroup);
        expect($firstnameError.html()).to.equal("First Name is Incorrect: Please enter no more than 19 char");
    });
    it("has enough characters", function () {
        $firstnameInput.val("a");
        $firstnameError.html("");
        callRegistrationInputHandler($firstnameInputGroup);
        expect($firstnameError.html()).to.equal("First Name is Incorrect: Please enter 2 upper case or lower case at least");
    });
    it("isn't invalid characters", function () {
        $firstnameInput.val("##");
        $firstnameError.html("");
        callRegistrationInputHandler($firstnameInputGroup);
        expect($firstnameError.html()).to.equal("First Name is Incorrect: Please enter upper case and lower case only");
    });
    it("is valid", function () {
        $firstnameInput.val("John");
        $firstnameError.html("");
        callRegistrationInputHandler($firstnameInputGroup);
        expect($firstnameError.html()).to.equal("First Name is Ok: Your data has been entered correctly");
    });
});

Adding lastname tests

The lastname tests are similar in nature too.

describe("registration-input last name", function () {
    /*
       Structure
       - .form-group
         - .starrq
         - .input-group
           - input
         - .inputstatus
           - .error
           - .feedback
    */
    function callRegistrationInputHandler(thisArg) {
        const registrationInputHandler = registration.eventHandler.registrationInput;
        registrationInputHandler.call(thisArg);
    }
    const $lastnameGroup = $(".form-group").has("[name='Last Name']");
    const $lastnameInputGroup = $lastnameGroup.find(".input-group");
    const $lastnameInput = $lastnameGroup.find("input");
    const $lastnameError = $lastnameGroup.find(".error");
    after(function () {
        $("#registration").trigger("reset");
    });
    it("isn't empty", function () {
        $lastnameInput.val("");
        $lastnameError.html("");
        callRegistrationInputHandler($lastnameInputGroup);
        expect($lastnameError.html()).to.equal("Last Name is Empty: Please enter data into this input");
    });
    it("isn't fake", function () {
        $lastnameInput.val("abbbc");
        $lastnameError.html("");
        callRegistrationInputHandler($lastnameInputGroup);
        expect($lastnameError.html()).to.equal("Last Name is Fake text: Please remove repetition");
    });
    it("isn't too long", function () {
        $lastnameInput.val("Too much text in the input field");
        $lastnameError.html("");
        callRegistrationInputHandler($lastnameInputGroup);
        expect($lastnameError.html()).to.equal("Last Name is Incorrect: Please enter no more than 19 char");
    });
    it("has enough characters", function () {
        $lastnameInput.val("a");
        $lastnameError.html("");
        callRegistrationInputHandler($lastnameInputGroup);
        expect($lastnameError.html()).to.equal("Last Name is Incorrect: Please enter 2 upper case or lower case at least");
    });
    it("isn't invalid characters", function () {
        $lastnameInput.val("##");
        $lastnameError.html("");
        callRegistrationInputHandler($lastnameInputGroup);
        expect($lastnameError.html()).to.equal("Last Name is Incorrect: Please enter upper case and lower case only");
    });
    it("is valid", function () {
        $lastnameInput.val("John");
        $lastnameError.html("");
        callRegistrationInputHandler($lastnameInputGroup);
        expect($lastnameError.html()).to.equal("Last Name is Ok: Your data has been entered correctly");
    });
});

Restructure the code

Now that we have tests in place, I can reorganise the code with no fear of breaking anything.

I’ve reorganised the if/else structure to flatten the complexity, with the successful validation right at the end.

    if (name === "First Name") {
        if (fakeReg.test(value)) {
            //...
        } else if (value.length > 19) {
            //...
        } else if (/^([a-zA-Z]{1})$/.test(value) === true) {
            //...
        } else if (/^([a-zA-Z]{1,16})+$/.test(value) !== true) {
            //...
        } else {
            //...
        }
    }

Create validation functions

To get started at reducing duplication, we use an easy reference to the input field.

        const input = $(this).find(".check, textarea");
        // var name = $(this).find(".check,textarea").attr("name");
        // var value = $(this).find(".check,textarea").val().trim();
        const name = input.name;
        const value = input.value.trim();

With that input field we can create the validation rule functions and use them in the if/else statements. I’ve tried to phrase the function rules in terms of positive things that we are looking for, so that when it’s not found we can show a suitable error message.

        function lessThanTwentyChars(input) {
            return input.value.length < 20;
        }
        function moreThanOneAlpha(input) {
            return !/^([a-zA-Z]{1})$/.test(input.value);
        }
        function onlyAlphaChars(input) {
            return /^([a-zA-Z]{1,})+$/.test(input.value);
        }
        if (fakeReg.test(value)) {
            //...
        } else if (!lessThanTwentyChars(input)) {
            //...
        } else if (!moreThanOneAlpha(input)) {
            //...
        } else if (!onlyAlphaChars(input)) {
            //...
        } else {
            //...
        }

Use validation rules to make validation functions

We can now create the check methods, for validate to use.

        const checkLessThanTwentyChars = validate.createValidator(lessThanTwentyChars, "Please enter no more than 19 char");
        const checkMoreThanOneAlpha = validate.createValidator(moreThanOneAlpha, "Please enter 2 upper case or lower case at least");
        const checkOnlyAlphaChars = validate.createValidator(onlyAlphaChars, "Please enter upper case and lower case only");

We can now validate using the validate.check method.
check method starting from the bottom, thanks to having those validation rule functions.

    if (name === "First Name") {
        if (fakeReg.test(value)) {
            validate.check($formGroup, {
                "First Name": [
                    validate.fn.checkFake
                ]
            });
        } else if (!lessThanTwentyChars(input)) {
            validate.check($formGroup, {
                "First Name": [checkLessThanTwentyChars]
            });
        } else if (!moreThanOneAlpha(input)) {
            validate.check($formGroup, {
                "First Name": [checkMoreThanOneAlpha]
            });
        } else {
            validate.check($formGroup, {
                "First Name": [checkOnlyAlphaChars]
            });
        }
    }

and lastly, we can join those separate rules together into a single array of rules.

    if (name === "First Name") {
        validate.check($formGroup, {
            "First Name": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                checkLessThanTwentyChars,
                checkMoreThanOneAlpha,
                checkOnlyAlphaChars
            ]
        });
    }

Updating the last name code

Because the last name section has the same error messages as the first name section, we can replace all of the last name code with the first name code instead.

    if (name === "Last Name") {
        validate.check($formGroup, {
            "Last Name": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                checkLessThanTwentyChars,
                checkMoreThanOneAlpha,
                checkOnlyAlphaChars
            ]
        });
    }

Summary

We added tests for firstname and lastname validations, simplified the validation structure, created validation rule functions, and used those rules with validate.check()

The code as it stands today is found at v0.0.25 in releases

Next time we use a similar technique to update the rest of the registration validations.

[Paul_Wilkins]

Removing duplication from registration validation is what we’re up to today. We will be doing the following:

• Add tests for remaining registration validations
• simplifying if/else and use validate.check
• creating rule functions to use for validation
• use validate.check and condense them together

Add tests for remaining registration validations

I have added tests for all of the remaining form fields dealt with by the registration input handler. Those being for phone number, city, email, and password.

Here are the phone number tests:

registration-input-phonenumber.test.js

describe("registration input phonenumber", function () {
    /*
       Structure
       - .form-group
         - .starrq
         - .input-group
           - input
         - .inputstatus
           - .error
           - .feedback
    */
    function callRegistrationInputHandler(thisArg) {
        const registrationInputHandler = registration.eventHandler.registrationInput;
        registrationInputHandler.call(thisArg);
    }
    const $phoneGroup = $(".form-group").has("[name='Phone Number']");
    const $phoneInputGroup = $phoneGroup.find(".input-group");
    const $phoneInput = $phoneGroup.find("input");
    const $phoneError = $phoneGroup.find(".error");
    after(function () {
        $("#registration").trigger("reset");
    });
    it("is empty", function () {
        $phoneInput.val("");
        callRegistrationInputHandler($phoneInputGroup);
        expect($phoneError.html()).to.equal("Phone Number is Empty: Please enter data into this input");
    });
    it("isn't a phone number", function () {
        $phoneInput.val("not phone number");
        callRegistrationInputHandler($phoneInputGroup);
        expect($phoneError.html()).to.equal("Phone Number is Incorrect: Please enter Phone Number correctly");
    });
    it("is a phone number", function () {
        $phoneInput.val("(1234)-567-8901");
        callRegistrationInputHandler($phoneInputGroup);
        expect($phoneError.html()).to.equal("Phone Number is Ok: Your Phone number has been entered correctly");
    });
});

Here are the city tests:

registration-input-city.test.js

describe("registration input city", function () {
    /*
       Structure
       - .form-group
         - .starrq
         - .input-group
           - input
         - .inputstatus
           - .error
           - .feedback
    */
    function callRegistrationInputHandler(thisArg) {
        const registrationInputHandler = registration.eventHandler.registrationInput;
        registrationInputHandler.call(thisArg);
    }
    const $cityGroup = $(".form-group").has("[name='Your City']");
    const $cityInputGroup = $cityGroup.find(".input-group");
    const $cityInput = $cityGroup.find("input");
    after(function () {
        $("#registration").trigger("reset");
    });
    it("value is empty", function () {
        $cityInput.val("");
        const $cityError = $cityGroup.find(".error");
        $cityError.html("");
        callRegistrationInputHandler($cityInputGroup);
        expect($cityError.html()).to.equal("Your City field is Empty!");
    });
    describe("value has content", function () {
        $cityInput.val("test value");
        const $cityError = $cityGroup.find(".error");
        $cityError.html("");
        callRegistrationInputHandler($cityInputGroup);
        expect($cityError.html()).to.equal("Your City field is OK!");
    });
});

Here are the email tests:

registration-input-email.test.js

describe("registration-input email", function () {
    /*
       Structure
       - .form-group
         - .starrq
         - .input-group
           - input
         - .inputstatus
           - .error
           - .feedback
    */
    function callRegistrationInputHandler(thisArg) {
        const registrationInputHandler = registration.eventHandler.registrationInput;
        registrationInputHandler.call(thisArg);
    }
    const $emailGroup = $(".form-group").has("[name='E-mail']");
    const $emailInputGroup = $emailGroup.find(".input-group");
    const $emailInput = $emailGroup.find("input");
    const $emailError = $emailGroup.find(".error");
    after(function () {
        $("#registration").trigger("reset");
    });
    it("is empty", function () {
        $emailInput.val("");
        callRegistrationInputHandler($emailInputGroup);
        expect($emailError.html()).to.equal("E-mail is Empty: Please enter data into this input");
    });
    it("has repetition", function () {
        $emailInput.val("abbbc");
        callRegistrationInputHandler($emailInputGroup);
        expect($emailError.html()).to.equal("E-mail is Fake text: Please remove repetition");
    });
    it("isn't valid", function () {
        $emailInput.val("test@example");
        callRegistrationInputHandler($emailInputGroup);
        expect($emailError.html()).to.equal("E-mail is Incorrect: Please enter it correctly");
    });
    it("is valid", function () {
        $emailInput.val("test@example.com");
        callRegistrationInputHandler($emailInputGroup);
        expect($emailError.html()).to.equal("E-mail is Ok: Your data has been entered correctly");
    });
});

And, here are the password tests:

registration-input-password.test.js

describe("registration-input password", function () {
    /*
       Structure
       - .form-group
         - .starrq
         - .input-group
           - input
         - .inputstatus
           - .error
           - .feedback
    */
    function callRegistrationInputHandler(thisArg) {
        const registrationInputHandler = registration.eventHandler.registrationInput;
        registrationInputHandler.call(thisArg);
    }
    const $passwordGroup = $(".form-group").has("[name='Password']");
    const $passwordInputGroup = $passwordGroup.find(".input-group");
    const $passwordInput = $passwordGroup.find("input");
    const $passwordError = $passwordGroup.find(".error");
    const $firstnameInput = $passwordGroup.closest("form").find("[name='First Name']");
    const $lastnameInput = $passwordGroup.closest("form").find("[name='Last Name']");
    const $cityInput = $passwordGroup.closest("form").find("[name='Your City']");
    after(function () {
        $("#registration").trigger("reset");
    });
    it("is empty", function () {
        $passwordInput.val("");
        callRegistrationInputHandler($passwordInputGroup);
        expect($passwordError.html()).to.equal("Password is Empty: Please enter data into this input");
    });
    it("has repetition", function () {
        $passwordInput.val("abbbc");
        callRegistrationInputHandler($passwordInputGroup);
        expect($passwordError.html()).to.equal("Password is Fake text: Please remove repetition");
    });
    it("shouldn't match firstname", function () {
        $firstnameInput.val("John");
        $passwordInput.val("John");
        callRegistrationInputHandler($passwordInputGroup);
        expect($passwordError.html()).to.equal("Password is Incorrect: Password shouldn't match first-name");
    });
    it("shouldn't match lastname", function () {
        $lastnameInput.val("Adams");
        $passwordInput.val("Adams");
        callRegistrationInputHandler($passwordInputGroup);
        expect($passwordError.html()).to.equal("Password is Incorrect: Password shouldn't match last-name");
    });
    it("shouldn't match city", function () {
        $cityInput.val("Chicago");
        $passwordInput.val("Chicago");
        callRegistrationInputHandler($passwordInputGroup);
        expect($passwordError.html()).to.equal("Password is Incorrect: Password shouldn't match city name");
    });
    it("should be at least 6 characters", function () {
        $passwordInput.val("12345");
        callRegistrationInputHandler($passwordInputGroup);
        expect($passwordError.html()).to.equal("Password is Incorrect: Please enter at lest 6 characters");
    });
    it("should be at most 12 characters", function () {
        $passwordInput.val("12345678901234567890");
        callRegistrationInputHandler($passwordInputGroup);
        expect($passwordError.html()).to.equal("Password is Incorrect: Please enter no more than 12 characters");
    });
    it("is valid", function () {
        $passwordInput.val("password");
        callRegistrationInputHandler($passwordInputGroup);
        expect($passwordError.html()).to.equal("Password is Ok: Your data has been entered correctly");
    });
});

Simplifying if/else code

Now that we have tests in place, we can futz about with the code making improvements. In this case, we will move the empty and repetition validations to the top of each if/else section, and the success message to the end.

The password code was the worst example, that started off as the following:

        if (name === "Password") {
            var pswReglow = /^([a-zA-Z0-9]{6,})+$/;
            var pswRegheigh = /^([a-zA-Z0-9]{13,})+$/; //13 or more occurences
            if (value === "") {
                //...
            } else if (value.length > 0) {
                if (fakeReg.test(value)) {
                    //...
                } else {
                    if (value === inputs["Your City"].value) {
                        //...
                    } else {
                        if (value === inputs["Last Name"].value) {
                            //...
                        } else {
                            if (value === inputs["First Name"].value) {
                                //...
                            } else {
                                if (!pswReglow.test(value)) {
                                    //...
                                } else {
                                    if (!pswRegheigh.test(value)) {
                                        //...
                                    } else {
                                        //...
                                    }
                                }
                            }
                        }
                    }
                }
            }
        }

The password validation code ends up being structured in a much simpler manner:

        if (name === "Password") {
            var pswReglow = /^([a-zA-Z0-9]{6,})+$/;
            var pswRegheigh = /^([a-zA-Z0-9]{13,})+$/; //13 or more occurences
            if (value === "") {
                //...
            } else if (fakeReg.test(value)) {
                //...
            } else if (value === inputs["First Name"].value) {
                //...
            } else if (value === inputs["Last Name"].value) {
                //...
            } else if (value === inputs["Your City"].value) {
                //...
            } else if (!pswReglow.test(value)) {
                //...
            } else if (pswRegheigh.test(value)) {
                //...
            } else {
                //...
            }
        }

Refactor to use validate.check instead

The rest of the work consists of:

• creating rule functions for each if condition
• using those rules to create validation functions
• replacing code with validate.check
• moving rule functions into each validation function

Create rule functions

The validation rule functions are as follows:

        function isPhoneNumber(input) {
            var phoneReg = /^\(?([0-9]{4})\)?([ .-]?)([0-9]{3})\2([0-9]{4})$/;
            return phoneReg.test(input.value);
        }
    //...
            // } else if (!phoneReg.test(value)) {
            } else if (!isPhoneNumber(input)) {
    //...
        function isEmail(input) {
            var emailReg = /^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/;
            return emailReg.test(input.value);
        }
    //...
            // } else if (!emailReg.test(value)) {
            } else if (!isEmail(input)) {
    //...
        function differentThanFirstname(input) {
            const form = input.form;
            const firstname = form.elements["First Name"];
            return firstname.value !== input.value;
        }
        function differentThanLastname(input) {
            const form = input.form;
            const lastname = form.elements["Last Name"];
            return lastname.value !== input.value;
        }
        function differentThanCity(input) {
            const form = input.form;
            const city = form.elements["Your City"];
            return city.value !== input.value;
        }
        function passwordAtLeastSix(input) {
            var pswReglow = /^([a-zA-Z0-9]{6,})+$/;
            return pswReglow.test(input.value);
        }
        function passwordBelowThirteen(input) {
            var pswRegheigh = /^([a-zA-Z0-9]{13,})+$/; //13 or more occurences
            return !pswRegheigh.test(input.value)
        }
    //...
            // } else if (value === inputs["First Name"].value) {
            } else if (!differentThanFirstname(input)) {
    //...
            // } else if (value === inputs["Last Name"].value) {
            } else if (!differentThanLastname(input)) {
    //...
            // } else if (value === inputs["Your City"].value) {
            } else if (!differentThanCity(input)) {
    //...
            // } else if (!pswReglow.test(value)) {
            } else if (!passwordAtLeastSix(input)) {
    //...
            // } else if (pswRegheigh.test(value)) {
            } else if (!passwordBelowThirteen(input)) {
    //...
        function matchesPassword(input) {
            const form = input.form;
            const password = form.elements["Password"];
            return password.value === input.value;
        }
    //...
            // } else if (value !== inputs.Password.value) {
            } else if (!matchesPassword(input)) {

The important thing about the rule checks is that they are all named as something that must occur, so that later on when the validator uses the same logic, it knows that on a fail that failure message needs to occur.

use validation.check for each validation

We can now transition over to using validate.check by creating a validator, using it in the if/else statements, before joining them together.

        // function isPhoneNumber(input) {
        //     var phoneReg = /^\(?([0-9]{4})\)?([ .-]?)([0-9]{3})\2([0-9]{4})$/;
        //     return phoneReg.test(input.value);
        // }
        const checkIsPhoneNumber = validate.createValidator(
            function (input) {
                var phoneReg = /^\(?([0-9]{4})\)?([ .-]?)([0-9]{3})\2([0-9]{4})$/;
                return phoneReg.test(input.value);
            }, "Please enter Phone Number correctly"
        );
        //...
        if (name === "Phone Number") {
            if (value === "") {
                return validate.check($formGroup, {
                    "Phone Number": [
                        validate.fn.isEmpty
                    ]
                });
            } else {
                return validate.check($formGroup, {
                    "Phone Number": [
                        checkIsPhoneNumber
                    ]
                });
            }
        }

I am returning the validate.check statement, as that prevents us from executing later code causing conflicts with the tests.

Now that all if/else conditions are converted to use validate.check, we can start combining some of them together, joining the validation rules together:

        if (name === "Phone Number") {
            return validate.check($formGroup, {
                "Phone Number": [
                    validate.fn.isEmpty,
                    checkIsPhoneNumber
                ]
            });
        }

and when all of the input validation conditions use validate.check, we can join those together too.

Replacing email and city with validate.check

The remaining input validations are all replaced with validate.check methods too.

        const checkIsEmail = validate.createValidator(
            function (input) {
                var emailReg = /^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/;
                return emailReg.test(input.value);
            }, "Please enter it correctly"
        );
//...
        if (name === "E-mail") {
            return validate.check($formGroup, {
                "E-mail": [
                    validate.fn.checkEmpty,
                    validate.fn.checkFake,
                    checkIsEmail
                ]
            });
        }
//...
        if (name === "Your City") {
            return validate.check($formGroup, {
                "Your City": [
                    validate.fn.checkEmpty
                ]
            });
        }

Slower steps when converting password code

With the password things get trickier, so moving slower is helpful.

The inputStatus code is a beneficial first step:

        if (name === "Password") {
            if (value === "") {
                inputStatus.warning($formGroup, name + " is Empty: Please enter data into this input");
            } else if (fakeReg.test(value)) {
                inputStatus.warning($formGroup, name + " is Fake text: Please remove repetition");
            } else if (!differentThanFirstname(input)) {
                inputStatus.warning($formGroup, name + " is Incorrect: Password shouldn't match first-name");
            } else if (!differentThanLastname(input)) {
                inputStatus.warning($formGroup, name + " is Incorrect: Password shouldn't match last-name");
            } else if (!differentThanCity(input)) {
                inputStatus.warning($formGroup, name + " is Incorrect: Password shouldn't match city name");
            } else if (!passwordAtLeastSix(input)) {
                inputStatus.warning($formGroup, name + " is Incorrect: Please enter at least 6 characters");
            } else if (!passwordBelowThirteen(input)) {
                inputStatus.warning($formGroup, name + " is Incorrect: Please enter no more than 12 characters");
            } else {
                inputStatus.ok($formGroup, name + " is Ok: Your data has been entered correctly");
            }

On the last section about the password being below thirteen, we can use validate.check on the last password error:

            } else if (!passwordBelowThirteen(input)) {
                return validate.check($formGroup, {
                    "Password": [
                        checkPasswordBelowThirteen
                    ]
                });
            }

But that results in the login password having trouble. login input password - password is fake: AssertionError: expected 'Password is Ok: Your data has been entered correctly' to equal 'Password is Fake text: Please remove repetition'

That kind of issue can only occur when the validate check function wants improvement.

    function check(inputGroup, validators) {
        const validationTypes = Object.assign(defaultValidators, validators);

The problem is that the defaultValidators variable ends up being the target. The default information gets changed each time we use validate.check, which is not what we want.

Instead, we want validationTypes to be a copy of the default, and to then update that copy with the extra validator. Here’s how we achieve that:

    function check(inputGroup, validators) {
        // const validationTypes = Object.assign(defaultValidators, validators);
        const validationTypes = Object.create(defaultValidators);
        Object.assign(validationTypes, validators);

and the tests all pass once again.

We can now move ahead with using validate.check on the remainder of the password code:

        if (name === "Password") {
            if (value === "") {
                return validate.check($formGroup, {
                    "Password": [
                        validate.fn.checkEmpty
                    ]
                });
            } else if (fakeReg.test(value)) {
                return validate.check($formGroup, {
                    "Password": [
                        validate.fn.checkFake
                    ]
                });
            } else if (!differentThanFirstname(input)) {
                return validate.check($formGroup, {
                    "Password": [
                        checkDifferentThanFirstname
                    ]
                });
            } else if (!differentThanLastname(input)) {
                return validate.check($formGroup, {
                    "Password": [
                        checkDifferentThanLastname
                    ]
                });
            } else if (!differentThanCity(input)) {
                return validate.check($formGroup, {
                    "Password": [
                        checkDifferentThanCity
                    ]
                });
            } else if (!passwordAtLeastSix(input)) {
                return validate.check($formGroup, {
                    "Password": [
                        checkPasswordAtLeastSix
                    ]
                });
            } else {
                return validate.check($formGroup, {
                    "Password": [
                        checkPasswordBelowThirteen
                    ]
                });
            }
        }

We can now add more pieces to the else clause, helping us to remove other earlier parts:

            // } else if (!differentThanCity(input)) {
            //     validate.check($formGroup, {
            //         "Password": [
            //             checkDifferentThanCity
            //         ]
            //     });
            // } else if (!passwordAtLeastSix(input)) {
            //     validate.check($formGroup, {
            //         "Password": [
            //             checkPasswordAtLeastSix
            //         ]
            //     });
            } else {
                return validate.check($formGroup, {
                    "Password": [
                        checkDifferentThanCity,
                        checkPasswordAtLeastSix,
                        checkPasswordBelowThirteen
                    ]
                });
            }
        }

Removing more of the if statments leaves us with only the validate.check code in the password section:

        if (name === "Password") {
            return validate.check($formGroup, {
                "Password": [
                    validate.fn.checkEmpty,
                    validate.fn.checkFake,
                    checkDifferentThanFirstname,
                    checkDifferentThanLastname,
                    checkDifferentThanCity,
                    checkPasswordAtLeastSix, checkPasswordBelowThirteen
                ]
            });
        }

The retype password code is the last part that gets the validate.check treatment.

        if (name === "Retype Password") {
            return validate.check($formGroup, {
                "Retype Password": [
                    validate.fn.checkEmpty,
                    checkMatchesPassword
                ]
            });
        }

Remove the if statements to join validations together

Now that all of the validate.check statements are there, with return statements helping to make the conversion easier, I can remove the if statement from around the retype password section of code:

        // if (name === "Retype Password") {
            return validate.check($formGroup, {
                "Retype Password": [
                    validate.fn.checkEmpty,
                    checkMatchesPassword
                ]
            });
        // }

We can now start to join the validate.check methods together:

        // if (name === "Password") {
        return validate.check($formGroup, {
            "Password": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                checkDifferentThanFirstname,
                checkDifferentThanLastname,
                checkDifferentThanCity,
                checkPasswordAtLeastSix, checkPasswordBelowThirteen
        //     ]
            ],
        // });
        // }
        // return validate.check($formGroup, {
            "Retype Password": [
                validate.fn.checkEmpty,
                checkMatchesPassword
            ]
        });

After condensing the other parts, we now have a single validate.check statement with easily adjusted validation sections for each section.

        const $formGroup = $(this).closest(".form-group");
        return validate.check($formGroup, {
            "First Name": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                checkLessThanTwentyChars,
                checkMoreThanOneAlpha,
                checkOnlyAlphaChars
            ],
            "Last Name": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                checkLessThanTwentyChars,
                checkMoreThanOneAlpha,
                checkOnlyAlphaChars
            ],
            "Phone Number": [
                validate.fn.checkEmpty,
                checkIsPhoneNumber
            ],
            "E-mail": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                checkIsEmail
            ],
            "Postal Address": [
                validate.fn.checkEmpty,
                checkPostalAddress
            ],
            "zip code": [
                validate.fn.checkEmpty,
                checkPostcode
            ],
            "Your City": [
                validate.fn.checkEmpty
            ],
            "Password": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                checkDifferentThanFirstname,
                checkDifferentThanLastname,
                checkDifferentThanCity,
                checkPasswordAtLeastSix, checkPasswordBelowThirteen
            ],
            "Retype Password": [
                validate.fn.checkEmpty,
                checkMatchesPassword
            ]
        });

Summary

We added tests for remaining registration validations, simplified if/else statements to use with validate.check, created rule functions to use for validation, and used validate.check to condense things together.

The code as it stands today is found at v0.0.26 in releases

Next time we remove duplication from the citylist click handler.

[Paul_Wilkins]

Removing duplication from … is what we’re up to today. We will be doing the following:

• adding tests for the citylist click handler
• removing duplication from citylist code
• making the code easier to understand

Refine jsInspect’s ability to find duplication

As the identifiers and variable names make it more difficult for duplication to be noticed, we can tell the jsInspect program to ignore identifiers and ignore variables.

We can update the inspect.bat batch file to help out, by adding -I to ignore different identifiers, and -L to ignore different literals.

inspect.bat

@echo off
npx jsinspect -t 20 registration3 --ignore lib -I -L

The inspection for duplication now finds a few more useful sets of duplication, resulting in 8 more sets of it to take care of.

Removing duplication from city click handler

The next set of duplication is found in the city click handler:

./registration3\registration.js:320,322
$(".form-group").find("#errorid").addClass('ok').removeClass('warning');
$(".form-group").find("#feedbackid").removeClass("glyphicon glyphicon-remove").addClass("glyphicon glyphicon-ok").removeClass("warning").addClass("ok");
$(".form-group").find("#cityRequired").addClass('ok').removeClass('warning');

./registration3\registration.js:324,326
$(".form-group").find("#errorid").html("Your " + name + " field is Empty !").removeClass("ok").addClass("warning");
$(".form-group").find("#feedbackid").removeClass("glyphicon glyphicon-ok").addClass("glyphicon glyphicon-remove").removeClass("ok").addClass("warning");
$(".form-group").find("#cityRequired").removeClass("ok").addClass("warning");

This is the code that occurs when a selection is made from the list of cities that appears when clicking on the city field.

Add tests for the expected behaviour

The code that needs tests is as follows:

    $(".citylist li").click(function() {
        var city = $(this).text().trim();
        var name = $("#your-city").attr("name");
        $("#your-city").val(city);
        $("#demo2").collapse("hide");
        if (city != "") {
            $(".form-group").find("#errorid").html(name + " is OK: Your data has been entered correctly");
            $(".form-group").find("#errorid").addClass('ok').removeClass('warning');
            $(".form-group").find("#feedbackid").removeClass("glyphicon glyphicon-remove").addClass("glyphicon glyphicon-ok").removeClass("warning").addClass("ok");
            $(".form-group").find("#cityRequired").addClass('ok').removeClass('warning');
        } else {
            $(".form-group").find("#errorid").html("Your " + name + " field is Empty !").removeClass("ok").addClass("warning");
            $(".form-group").find("#feedbackid").removeClass("glyphicon glyphicon-ok").addClass("glyphicon glyphicon-remove").removeClass("ok").addClass("warning");
            $(".form-group").find("#cityRequired").removeClass("ok").addClass("warning");
        }
    });

The things that the code does that need testing is that the city value is updated, the list of cities is collapsed, and the city value is validated. Quite how the city becomes empty is a mystery - I’ll find out more when setting up the tests.

The first citylist test

The first citylist test helps us to ensure that we are testing something related to the code that we are going to update.

citylist-click.test.js

describe("registration input city", function () {
    /*
       Structure
       - ul.citylist
         - li
    */
    function callCitylistClickHandler(thisArg) {
        $(".citylist li").first().trigger("click");
    }
    after(function () {
        $("#demo2").collapse("hide");
    });
    it("updates the city", function () {
        $("#your-city").val("");
        callCitylistClickHandler();
        expect($("#your-city").val()).to.equal("London");
    });
});

Separate the event assignment from event handler

Now that we have something being tested, we can update the citylist code so that it is easier to test. We separate the click handler from the citylist assignment, and make the click handler available for testing.

    // $(".citylist li").click(function () {
    function citylistClickHandler() {
        //...
    }
    $(".citylist li").click(citylistClickHandler);
    //...
    return {
        eventHandler: {
            //...
            citylistClick: citylistClickHandler,
            termsClick: termsClickHandler
        }
    };

Test using the event handler function

We can now access that citylistClickHandler from the test, without needing to trigger the click event. It’s safer to test things when less side-effects are likely to occur.

    function callCitylistClickHandler(thisArg) {
        // $(".citylist li").first().trigger("click");
        const citylistClickHandler = registration.eventHandler.citylistClick;
        const firstItem = $(".citylist li").first().get(0);
        const evt = {
            target: firstItem
        };
        citylistClickHandler.call(evt.target, evt);
    }

We can now carry on to test the other things that happen when we use citylistClickHandler.

Adding further citylist click handler tests

Testing the collapse is something that I’ll skip over. Testing DOM behaviour is typically a bad idea, but here we have an animation too.

As we are using bootstrap, it’s enough to check that the collapse class names are appropriately on the elements.

    it("can collapses the city list", function () {
        expect($("#citylist").attr("class")).to.contain("collapse");
    });

Testing the citylist input

The next behaviour of the code is as follows:

        if (city != "") {
            $(".form-group").find("#errorid").html(name + " is Ok: Your data has been entered correctly");
            $(".form-group").find("#errorid").addClass('ok').removeClass('warning');
            $(".form-group").find("#feedbackid").removeClass("glyphicon glyphicon-remove").addClass("glyphicon glyphicon-ok").removeClass("warning").addClass("ok");
            $(".form-group").find("#cityRequired").addClass('ok').removeClass('warning');
        } else {

The #errorid and #feedbackid look to be lazy attempts to access the city-specific error and feedback areas. We’ll test specifically on those elements for now, until we can ensure that the need for #errorid and #feedback can be removed.

This code is easy to test. We won’t eventually need to test all features because we plan to replace it with inputStatus code instead, which already has reliable behaviour.

Until we do use inputStatus though, we’ll need to have tests in place for each detail, until we transition things over to using the inputStatus code.

    $cityInput = $("#registration [name='Your City']");
    $cityGroup = $cityInput.closest(".form-group");
    $cityError = $cityGroup.find(".error");
//...
    it("shows no error", function () {
        $cityError.html("");
        callCitylistClickHandler();
        expect($cityError.html()).to.equal("Your City is Ok: Your data has been entered correctly");
    });
    it("adds ok to error", function () {
        $("#errorid").removeClass("ok");
        callCitylistClickHandler();
        expect($("#errorid").attr("class")).to.contain("ok");
    });
    it("removes warning from error", function () {
        $("#errorid").addClass("warning");
        callCitylistClickHandler();
        expect($cityError.attr("class")).to.not.contain("warning");
    });
    it("adds glyphicon to feedback", function () {
        $("#feedbackid").removeClass("glyphicon");
        callCitylistClickHandler();
        expect($("#feedbackid").attr("class")).to.contain("glyphicon");
    });
    it("removes glyphicon-remove from feedback", function () {
        $("#feedbackid").addClass("glyphicon-remove");
        callCitylistClickHandler();
        expect($("#feedbackid").attr("class")).to.not.contain("glyphicon-remove");
    });
    it("adds glyphicon-ok to feedback", function () {
        $("#feedbackid").removeClass("glyphicon-ok");
        callCitylistClickHandler();
        expect($("#feedbackid").attr("class")).to.contain("glyphicon-ok");
    });
    it("removes warning from feedback", function () {
        $("#feedbackid").addClass("warning");
        callCitylistClickHandler();
        expect($("#feedbackid").attr("class")).to.not.contain("warning");
    });
    it("adds ok to feedback", function () {
        $("#feedbackid").removeClass("ok");
        callCitylistClickHandler();
        expect($("#feedbackid").attr("class")).to.contain("ok");
    });
    it("adds ok to required", function () {
        $("#cityRequired").removeClass("ok");
        callCitylistClickHandler();
        expect($("#cityRequired").attr("class")).to.contain("ok");
    });
    it("removes warning from required", function () {
        $("#cityRequired").addClass("warning");
        callCitylistClickHandler();
        expect($("#cityRequired").attr("class")).to.not.contain("warning");
    });

Testing the empty input

The last part of the code to test is when the input is empty.

        } else {
            $(".form-group").find("#errorid").html(name + " is Empty: Please enter data into this input").removeClass("ok").addClass("warning");
            $(".form-group").find("#feedbackid").removeClass("glyphicon glyphicon-ok").addClass("glyphicon glyphicon-remove").removeClass("ok").addClass("warning");
            $(".form-group").find("#cityRequired").removeClass("ok").addClass("warning");
        }

I haven’t been able to figure out how to get this code to run, without there being an empty citylist item.

Because normal behaviour isn’t capable of causing this part of the code to run, it’s fair to assume that it’s only accidentally here from a copy/paste of other similar code.

However, there is a way to test this code, and that is to add an empty citylist item and use that for the test instead.

We can add an empty item before testing, get that empty item before each test, then remove it after all of the tests have been done.

    describe("On an error", function () {
        let emptyItem;
        beforeEach(function () {
            $(".citylist").append("<li></li>");
            emptyItem = $(".citylist li").last().get(0);
        });
        afterEach(function () {
            $(".citylist li:last").remove();
        });
        it("shows an error message", function () {
            $cityError.html("");
            callCitylistClickHandler(emptyItem);
            expect($cityError.html()).to.equal("Your City is Empty: Please enter data into this input");
        });
        it("shows warning on error", function () {
            $cityError.removeClass("warning");
            callCitylistClickHandler(emptyItem);
            expect($cityError.attr("class")).to.contain("warning");
        });
    });

The tests are all in place, we can now improve the code.

Improve the citylist click code

Can we remove the errorid, feedbackid, and cityrequired references? We sure can, by using the standard .error, .feedback, and .starrq references that we’ve used before.

Here’s an example of replacing #errorid with $cityError:

    const $cityInput = $("#registration [name='Your City']");
    const $cityGroup = $cityInput.closest(".form-group");
    const $cityError = $cityGroup.find(".error");
    const $cityFeedback = $cityGroup.find(".feedback");
    const $cityRequired = $cityGroup.find(".starrq");
    //...
    it("adds ok to error", function () {
        // $("#errorid").removeClass("ok");
        $cityError.removeClass("ok");
        callCitylistClickHandler(firstItem);
        // expect($("#errorid").attr("class")).to.contain("ok");
        expect($cityError.attr("class")).to.contain("ok");
    });

Here’s an example of replacing #feedbackid with $cityFeedback:

    it("adds glyphicon to feedback", function () {
        // $("#feedbackid").removeClass("glyphicon");
        $cityFeedback.removeClass("glyphicon");
        callCitylistClickHandler(firstItem);
        // expect($("#feedbackid").attr("class")).to.contain("glyphicon");
        expect($cityFeedback.attr("class")).to.contain("glyphicon");
    });

And, here is an example of replacing #cityRequired with $cityRequired instead:

    it("adds ok to required", function () {
        // $("#cityRequired").removeClass("ok");
        $cityRequired.removeClass("ok");
        callCitylistClickHandler(firstItem);
        // expect($("#cityRequired").attr("class")).to.contain("ok");
        expect($cityRequired.attr("class")).to.contain("ok");
    });

Can we now remove errorid, feedbackid, and cityrequired from the HTML code?

Searching all of the files shows that nothing refers to them now, so we can clean up by removing those unneeded identifiers.

    <!--<span id="cityRequired" class="glyphicon glyphicon-star starrq"></span> City:</label>-->
    <span class="glyphicon glyphicon-star starrq"></span> City:</label>
<!-- ... -->
    <!--<span id="errorid" class="error">Please click on the input field for the list of cities.</span>-->
        <span class="error">Please click on the input field for the list of cities.</span>
<!-- ... -->
    <!--<span id="feedbackid" class="feedback"></span>-->
        <span class="feedback"></span>

We can now carry on to improve the code JS too.

Use inputStatus to simplify code

We can now use inputStatus for the error messages.

        $form = $(this).closest("form");
        $cityInput = $form.find("[name='Your City']");
        $cityGroup = $cityInput.closest(".form-group");
        if (city != "") {
            // $(".form-group").find("#errorid").html(name + " is Ok: Your data has been entered correctly");
            // $(".form-group").find("#errorid").addClass('ok').removeClass('warning');
            // $(".form-group").find("#feedbackid").removeClass("glyphicon glyphicon-/ remove").addClass("glyphicon glyphicon-ok").removeClass("warning").addClass("ok");
            // $(".form-group").find("#cityRequired").addClass('ok').removeClass('warning');
            inputStatus.ok($cityGroup, name + " is Ok: Your data has been entered correctly");
        } else {
            // $(".form-group").find("#errorid").html(name + " is Empty: Please enter data into this input").removeClass("ok").addClass("warning");
            // $(".form-group").find("#feedbackid").removeClass("glyphicon glyphicon-ok").addClass("glyphicon glyphicon-remove").removeClass("ok").addClass("warning");
            // $(".form-group").find("#cityRequired").removeClass("ok").addClass("warning");
            inputStatus.warning($cityGroup, name + " is Empty: Please enter data into this input");
        }

and we can move the empty test to the top, making this more consistent with other similar code:

        if (city === "") {
            inputStatus.warning($cityGroup, name + " is Empty: Please enter data into this input");
        } else {
            inputStatus.ok($cityGroup, name + " is Ok: Your data has been entered correctly");
        }

Cleaning up the rest of the citylist code

Here’s the section of code that we are now going to improve:

    function citylistClickHandler() {
        var city = $(this).text().trim();
        var name = $("#your-city").attr("name");
        $("#your-city").val(city);
        $("#demo2").collapse("hide");
        $form = $(this).closest("form");
        $cityInput = $form.find("[name='Your City']");
        $cityGroup = $cityInput.closest(".form-group");
    //...

We can reorganise this code so that the your-city identifier isn’t needed. When dealing with forms, best-practice is to use the named form fields instead.

    function citylistClickHandler() {
        const $form = $(this).closest("form");
        const $cityInput = $form.find("[name='Your City']");
        const $cityGroup = $cityInput.closest(".form-group");
        const value = $(this).text().trim();
        $cityInput.val(value);
        $("#demo2").collapse("hide");

Finding a better name than demo2

That demo2 isn’t good either, but not because it’s an identifer. It’s because demo2 tells us nothing about what is being affected.

We can search for all references to demo2, and we find that it’s only in the HTML as a collapse term, and that one JS reference too.

We can easily rename demo2 to have a much more instructive name of citylist instead.

    <!--<div data-toggle="collapse" data-target="#demo2">-->
        <div data-toggle="collapse" data-target="#citylist">
            <!-- ... -->
        <!--<div id="demo2" class="collapse">-->
            <div id="citylist" class="collapse">

        // $("#demo2").collapse("hide");
        $("#citylist").collapse("hide");

That gives us final citylist click code that is simpler and easier to understand:

    function citylistClickHandler() {
        const $form = $(this).closest("form");
        const $cityInput = $form.find("[name='Your City']");
        const $cityGroup = $cityInput.closest(".form-group");

        const value = $(this).text().trim();
        $cityInput.val(value);
        $("#demo2").collapse("hide");

        validate.check($cityGroup, {
            "Your City": [validate.fn.checkEmpty]
        });
    }
    $(".citylist li").click(citylistClickHandler);

Summary

We added tests for the citylist click handler, and removed duplication from citylist code to make the code easier to understand.

The code as it stands today is found at v0.0.27 in releases

Next time we puzzle over similar differences between the login and registration code.

[Paul_Wilkins]

Removing duplication from the login and registration submit functions is what we’re up to today. We will be doing the following:

• simplifying the login submit code
• simplifying the registration submit code
• renaming input-check to check
• extracting out a common function for both
• moving that common code to validate module

Duplication found in registration and login submit code

The jsInspect code finds duplication existing in these locations:

./registration3\login.js:22,26
    inputStatus.feedbackWarning(inputGroup);
}
function loginSubmitHandler(evt) {
    $("#login .form-group").has(".input-check").each(function validateInput() {
        var inputName = $(this).find(".input-check").attr("name");

./registration3\registration.js:476,480
$('#terms').click(termsClickHandler);

function registrationSubmitHandler(evt) {
    $('.form-group').has(".check").each(function validateGroup() {
        var $requiredField = $(this).find("input, textarea");

Simplify the login submit code

Looking at the login click function, we now have validate.check to help simplify the code that’s there.

First, I update the messages so that they follow the same format as the other messages. The most reliable way to do that is to update the tests first, which helps to confirm that we do have tests in place:

login-submit.test.js

        // expect($emailError.html()).to.equal("Your E-mail is OK");
        expect($emailError.html()).to.equal("E-mail is Ok: Your data has been entered correctly");
//...
        // expect($emailError.html()).to.equal("Your E-mail is empty");
        expect($emailError.html()).to.equal("E-mail is Empty: Please enter data into this input");

The valid email test also wants to have not just any value, but an actual email address too.

    it("email has value, resets email error", function () {
        //...
        $emailInput.val("test@example.com");

and then update the code so that the tests pass:

            if (trimmedValue !== "") {
                // removeError(this, "Your " + inputName + " is OK");
                removeError(this, inputName + " is Ok: Your data has been entered correctly");
            } else {
                // addError(this, "Your " + inputName + " is empty");
                addError(this, inputName + " is Empty: Please enter data into this input");
                evt.preventDefault();
            }

Next we move the success part down to the bottom:

            if (trimmedValue === "") {
                addError(this, inputName + " is Empty: Please enter data into this input");
                evt.preventDefault();
            } else {
                removeError(this, inputName + " is Ok: Your data has been entered correctly");
            }

and now we can use validate.check to replace that code, allowing us to delete the addError and removeError functions:

    // function removeError(inputGroup, message) {
    //     inputStatus.errorOk(inputGroup, message);
    //     inputStatus.feedbackOk(inputGroup);
    // }
    // function addError(inputGroup, message) {
    //     inputStatus.errorWarning(inputGroup, message);
    //     inputStatus.feedbackWarning(inputGroup);
    // }
//...
            // var inputName = $(this).find(".input-check").attr("name");
            var trimmedValue = $(this).find(".input-check").val().trim();
            validate.check(this);
            if (trimmedValue === "") {
                evt.preventDefault();
            }

And lastly, instead of checking that the value is empty, a more generic solution is to check if the form has a warning class before preventing the default behaviour.

            // var trimmedValue = $(this).find(".input-check").val().trim();
            validate.check(this);
            if ($("#login").has(".warning")) {
                evt.preventDefault();
            }

Simplify the registration submit code

We can now simplify and reorganise the registration click handler so that it’s as close as possible to the login submit handler:

Here is how the registration click handler looks before we begin:

    function registrationSubmitHandler(evt) {
        $('.form-group').has(".check").each(function validateGroup() {
            var $requiredField = $(this).find("input, textarea");
            var name = $requiredField.attr("name");
            var value = $requiredField.val().trim();
            if (value === "") {
                inputStatus.warning(this, name + " is empty!");
            }
        });
        updateTerms();
        if ($(".inputstatus .warning").length !== 0) {
            evt.preventDefault();
        }
    }
    $("#registration").on("submit", registrationSubmitHandler);

We look to have suitable tests in place for this code, so we can now update it in the same way as we did with the login code:

Make check and input-check consistent

Currently the check classname is used for required registration form fields, and input-check is used for required login and change-password fields.

It will be easier to remove duplication when the same system is used across those different forms, so renaming input-check to check is what we will do.

index.html / tests.html

        <!--<input type="email" class="form-control borderbottom input-check" id="emailm1" placeholder="Enter email" name="E-mail">-->
            <input type="email" class="form-control borderbottom check" id="emailm1" placeholder="Enter email" name="E-mail">
<!-- ... -->
        <!--<input type="password" class="form-control borderbottom input-check" id="pwdm1" placeholder="Enter password" name="Password">-->
            <input type="password" class="form-control borderbottom check" id="pwdm1" placeholder="Enter password" name="Password">
<!-- ... -->
        <!--<input type="email" class="form-control borderbottom input-check" id="emailm2" placeholder="Enter email" name="E-mail">-->
            <input type="email" class="form-control borderbottom check" id="emailm2" placeholder="Enter email" name="E-mail">
<!-- ... -->
        <!--<input type="password" class="form-control borderbottom input-check" id="pwdm2" 
            placeholder="Type password" name="Password">-->
            <input type="password" class="form-control borderbottom check" id="pwdm2" 
            placeholder="Type password" name="Password">
<!-- ... -->
        <!--<input type="password" class="form-control borderbottom input-check" id="pwdmre2" 
            placeholder="Retype password" name="Password Retype">-->
            <input type="password" class="form-control borderbottom check" id="pwdmre2" 
            placeholder="Retype password" name="Password Retype">

That causes some tests to break, so we update the login and change-password code to make the tests pass again.

login.js

        // $("#login .form-group").has(".input-check").each(function validateInput() {
        $("#login .form-group").has(".check").each(function validateInput() {

change-password.js

    function passwordSubmitHandler(evt) {
        // $("#changepw .form-group").has(".input-check").each(function() {
        $("#changepw .form-group").has(".check").each(function() {
            // var trimmedValue = $(this).find(".input-check").val().trim();
            var trimmedValue = $(this).find(".check").val().trim();
            // var inputName = $(this).find(".input-check").attr("name");
            var inputName = $(this).find(".check").attr("name");

input-status.js

    function resetForm($form) {
        $form.find(".form-group").each(function() {
            // var inputName = $(this).find(".input-check").attr("name");
            var inputName = $(this).find(".check").attr("name");
            //...

and the tests have only two sections that refer to input-check too:

inputstatus.test.js

    describe("resetForm", function () {
        $form = $("form").eq(1);
        // $formGroups = $form.find(".form-group").has(".input-check");
        $formGroups = $form.find(".form-group").has(".check");
        $group1 = $formGroups.eq(0);
        $group2 = $formGroups.eq(1);
        // $name1 = $group1.find(".input-check").attr("name");
        $name1 = $group1.find(".check").attr("name");
        // $name2 = $group2.find(".input-check").attr("name");
        $name2 = $group2.find(".check").attr("name");
        $error1 = $group1.find(".error");
        $error2 = $group2.find(".error");

login-submit.test.js

    it("email is empty, shows email error", function () {
        const $emailGroup = $("#login .form-group").eq(1);
        // const $emailInput = $emailGroup.find(".input-check").first();
        const $emailInput = $emailGroup.find(".check").first();
        const $emailError = $emailGroup.find(".error");

Make the empty message consistent

We should now make the empty message consistent with the way that ew’ve use the empty message in previous code:

registration-submit.test.js

            it("shows the error text", function () {
                $firstnameError.html("");
                registrationSubmitHandler(fakeEvt);
                // expect($firstnameError.html()).to.equal("First Name is empty!");
                expect($firstnameError.html()).to.equal("First Name is Empty: Please enter data into this input");
            });

registration.js

            if (value === "") {
                // inputStatus.warning(this, name + " is empty!");
                inputStatus.warning(this, name + " is Empty: Please enter data into this input");
            }

Replace inputStatus with validate.check

We can now use validate.check instead of the inputStatus code:

    function registrationSubmitHandler(evt) {
        $('#registration .form-group').has(".check").each(function validateGroup() {
            const $requiredField = $(this).find("input, textarea");
            const name = $requiredField.attr("name");
            // const value = $requiredField.val().trim();
            // if (value === "") {
            //     inputStatus.warning(this, name + " is Empty: Please enter data into this input");
            // }
            const validations = [validate.fn.checkEmpty];
            validate.check(this, Object.fromEntries([
                [name, validations]
            ]));
        });

Restrict warning check to the form

Next, we can ensure that checking for warnings only occurs within the registration form itself.

        // if ($(".inputstatus .warning").length !== 0) {
        if ($("#registration .warning").length !== 0) {
            evt.preventDefault();
        }

I can now imagine a potential future, where some of this code is in the validate section, so that it can be used both by the registration and the login code.

Move common code out to a separate function

We can now move that submitValidation out to a separate function.

    function validateFormgroups() {
        $('#registration .form-group').has(".check").each(function validateGroup() {
            const $requiredField = $(this).find("input, textarea");
            const name = $requiredField.attr("name");
            const validations = [validate.fn.checkEmpty];
            validate.check(this, Object.fromEntries([
                [name, validations]
            ]));
        });
    }
    function registrationSubmitHandler(evt) {
        validateFormgroups();
        updateTerms();
        //...
    }

I can now pass a function argument for the #registration piece:

    // function validateFormgroups() {
    function validateFormgroups(form) {
        // $('#registration .form-group').has(".check").each(function validateGroup() {
        $(form).find('.form-group').has(".check").each(function validateGroup() {

That validateFormgroups can be moved into the validate code. It will be good as a checkFormEmpty function.

    function checkFormEmpty(form) {
        $(form).find(".form-group").has(".check").each(function validateGroup() {
            const $requiredField = $(this).find("input, textarea");
            const name = $requiredField.attr("name");
            const validations = [checkEmpty];
            check(this, Object.fromEntries([
                [name, validations]
            ]));
        });
    }
    return {
        createValidator,
        check,
        checkFormEmpty,

We can now update the registration and the login code, to use validate.checkFormEmpty instead.

registration.js

    function registrationSubmitHandler(evt) {
        // validateFormgroups("#registration");
        validate.checkFormEmpty("#registration");
        updateTerms();
        if ($("#registration .warning").length !== 0) {
            evt.preventDefault();
        }
    }

login.js

function loginSubmitHandler(evt) {
//        $("#login .form-group").has(".check").each(function validateInput() {
//            validate.check(this);
//        });
        validate.checkFormEmpty("#login");
        if ($("#login .warning").length !== 0) {
            evt.preventDefault();
        }
    }

Summary

We simplified the login and registration submit code, renamed input-check to check, extracted out a common function for both, and moved that common code to validate module.

The code as it stands today is found at v0.0.28 in releases

Next time we investigate duplication between our new validate function and the citylist click handler.

[Paul_Wilkins]

Removing duplication from checking empty inputs, and also from inputStatus, is what we’re up to today. We will be doing the following:

• removing duplication between citylist and validate code
• removing duplication from inputStatus feedback code
• also removing duplication from inputStatus error and required

Duplication found between citylist and validate code

./registration3\registration.js:313,316
function citylistClickHandler() {
    const $form = $(this).closest("form");
    const $cityInput = $form.find("[name='Your City']");
    const $cityGroup = $cityInput.closest(".form-group");

./registration3\validate.js:88,91
$(form).find(".form-group").has(".check").each(function validateGroup() {
    const $requiredField = $(this).find("input, textarea");
    const name = $requiredField.attr("name");
    const validations = [checkEmpty];

Making the duplication easier to understand

I’m puzzled about what duplication there is in that code. Perhaps if we give the input fields the same make things might become clearer.

registration.js

function citylistClickHandler() {
    const $form = $(this).closest("form");
    // const $cityInput = $form.find("[name='Your City']");
    const $inputField = $form.find("[name='Your City']");
    // const $cityGroup = $cityInput.closest(".form-group");
    const $cityGroup = $inputField.closest(".form-group");
//...
        // $cityInput.val(value);
        $inputField.val(value);

validate.js

$(form).find(".form-group").has(".check").each(function validateGroup() {
    // const $requiredField = $(this).find("input, textarea");
    const $inputField = $(this).find("input, textarea");
    // const name = $requiredField.attr("name");
    const name = $inputField.attr("name");

Extract check field code

The citylist code checks if a field is empty, and the validate code checks if a lot of fields are empty.

Some of the validate code can be extracted out to a separate checkFieldEmpty function so that we can make it available from the validate code, and use it from the registration code as well.

We add a test to validate.test.js for the expected behaviour of validate.checkFieldEmpty:

    describe("checks empty", function () {
        it("checks a field is empty", function () {
            emailInput.value = "";
            $emailError.removeClass("warning");
            validate.checkFieldEmpty(emailGroup);
            expect($emailError.attr("class")).to.contain("warning");
        });
        it("checks a field is not empty", function () {
            emailInput.value = "test@example.com";
            $emailError.removeClass("ok");
            validate.checkFieldEmpty(emailGroup);
            expect($emailError.attr("class")).to.contain("ok");
        });
    });

and we create the checkFieldEmpty function.

    function checkFieldEmpty(formGroup) {
        const $inputField = $(formGroup).find("input, textarea");
        const name = $inputField.attr("name");
        const validations = [checkEmpty];
        check(formGroup, Object.fromEntries([
            [name, validations]
        ]));
    }
    function checkFormEmpty(form) {
        $(form).find(".form-group").has(".check").each(function validateGroup() {
            // const $inputField = $(this).find("input, // textarea");
            // const name = $inputField.attr("name");
            // const validations = [checkEmpty];
            // check(this, Object.fromEntries([
            //     [name, validations]
            // ]));
            checkFieldEmpty(this);
        });
    }
    return {
        createValidator,
        check,
        checkFieldEmpty,
        checkFormEmpty,

Let’s now try and use that checkFieldEmpty function from the registration code:

registration.js

        // validate.check($cityGroup, {
        //     "Your City": [validate.fn.checkEmpty]
        // });
        validate.checkEmptyField($cityGroup);

The tests all still pass, and the duplication is no longer found.

Duplication found in inputStatus

The next issue of duplication is in the inputStatus code:

./registration3\input-status.js:29,37
    $feedback.removeClass("glyphicon glyphicon-ok");
    setNone($feedback);
}
function feedbackOk(inputGroup) {
    const $feedback = $(inputGroup).find(".feedback");
    feedbackNone(inputGroup);
    $feedback.addClass("glyphicon glyphicon-ok");
    setOk($feedback);
}

./registration3\input-status.js:35,43
    $feedback.addClass("glyphicon glyphicon-ok");
    setOk($feedback);
}
function feedbackWarning(inputGroup) {
    const $feedback = $(inputGroup).find(".feedback");
    feedbackNone(inputGroup);
    $feedback.addClass("glyphicon glyphicon-remove");
    setWarning($feedback);
}

After trying a few different things, success is found by using a separate setFeedback function, and passing “none”, “ok”, or “warning” to it.

    function setFeedback(inputGroup, type) {
        const $feedback = $(inputGroup).find(".feedback");
        const warningClass = "glyphicon glyphicon-remove";
        const okClass = "glyphicon glyphicon-ok";
        setNone($feedback, warningClass + " " + okClass);
        if (type === "ok") {
            setOk($feedback, okClass);
        }
        if (type === "warning") {
            setWarning($feedback, warningClass);
        }
    }
//...
    function feedbackNone(inputGroup) {
        setFeedback(inputGroup, "none");
    }
    function feedbackOk(inputGroup) {
        setFeedback(inputGroup, "ok");
    }
    function feedbackWarning(inputGroup) {
        setFeedback(inputGroup, "warning");
    }

This has me wanting to do the same thing for error and required now.

Pass ok or warning to a setError function

Instead of having the errorOk and errorWarning functions duplicate things, we can pass ok or warning to a separate setError function.

    function setError(inputGroup, type, message) {
        const $error = $(inputGroup).find(".error");
        $error.html(message);
        if (type === "ok") {
            $error.css("color", "green");
            setOk($error);
        }
        if (type === "warning") {
            $error.css("color", "red");
            setWarning($error);
        }
    }
//...
    function errorOk(inputGroup, message) {
        setError(inputGroup, "ok", message);
    }
    function errorWarning(inputGroup, message) {
        setError(inputGroup, "warning", message);
    }

And the same can be done for the required status too.

    function setRequired(inputGroup, type) {
        const $required = $(inputGroup).find(".starrq");
        if (type === "ok") {
            setOk($required);
        }
        if (type === "warning") {
            setWarning($required);
        }
    }
//...
    function requiredOk(inputGroup) {
        setRequired(inputGroup, "ok");
    }
    function requiredWarning(inputGroup) {
        setRequired(inputGroup, "warning");
    }

Much of the duplication in the inputStatus code has now been removed.

Summary

We removed duplication from between citylist and validate code, from inputStatus feedback code, and also from inputStatus error and required code too.

The code as it stands today is found at v0.0.29 in releases

Next time we take care of the largest amount of duplication of all, the mouseenter and mouseleave code at the top of the registration code.

[Paul_Wilkins]

Removing duplication from the mouseenter and mouseleave event handlers is what we’re up to today. We will be doing the following:

• investigating the mouseenter and mouseleave duplication
• using CSS to replace one set of it
• refining that CSS to replace all of it
• deleting the needed mouseenter and mouseleave code

Working through duplication from the start

The sets of duplication look to be getting less and less vital, and there’s still lots of duplication at the top of the registration code. That’s because we’ve been working from the end of the duplication list towards the beginning.

As there are only five sets of duplication reported now, let’s now work from the start of the list and see what’s waiting for us there at the top.

43 instances of duplication

The first set of duplication that jsInspect finds is all at the start of the registration code:

Match - 43 instances

./registration3\registration.js:9,12
$(".fst-name-glyph").on("mouseenter", function() {
    $(".fst-name-glyph").css("background-color", "darkred");
    $(".fst-name-glyph").css("color", "orange");
});

./registration3\registration.js:13,16
$(".fst-name-glyph").on("mouseleave", function() {
    $(".fst-name-glyph").css("background-color", "darkgreen");
    $(".fst-name-glyph").css("color", "yellow");
});

./registration3\registration.js:24,27
$(".fst-name-field").on("mouseenter", function() {
    $(".fst-name-glyph").css("background-color", "darkred");
    $(".fst-name-glyph").css("color", "orange");
});

I’ve only shown 3 of the 43 instances, but they are all easily visible at the start of the registration code.

Use CSS instead

Those sections of code are designed to achieve a hover presentation. JavaScript is almost always the worst way to do that, with CSS being the best way instead.

Here is a slightly reorganised version of the full code for the firstname color changes:

    $(".fst-name-glyph").on("mouseenter", function() {
        $(".fst-name-glyph").css("background-color", "darkred");
        $(".fst-name-glyph").css("color", "orange");
        $(".fst-name-field").css("background-color", "pink");
    });
    $(".fst-name-field").on("mouseenter", function() {
        $(".fst-name-glyph").css("background-color", "darkred");
        $(".fst-name-glyph").css("color", "orange");
        $(".fst-name-field").css("background-color", "pink");
    });

    $(".fst-name-glyph").on("mouseleave", function() {
        $(".fst-name-glyph").css("background-color", "darkgreen");
        $(".fst-name-glyph").css("color", "yellow");
        $(".fst-name-field").css("background-color", "lightgreen");
    });
    $(".fst-name-field").on("mouseleave", function() {
        $(".fst-name-glyph").css("background-color", "darkgreen");
        $(".fst-name-glyph").css("color", "yellow");
        $(".fst-name-field").css("background-color", "lightgreen");
    });

As fst-name-glyph and fst-name-field are both the only things inside of the input-group class, we can use input-group instead.

Here is how the CSS looks for the first name section:

.input-group .fst-name-glyph {
    width:75px;
    background-color: darkgreen;
    color: yellow;
}
.input-group .fst-name-field {
    background-color: lightgreen;
}
.input-group:hover .fst-name-glyph {
    width:75px;
    background-color: darkred;
    color: orange;
}
.input-group:hover .fst-name-field {
    background-color: pink;
}

It gets better than that though. Instead of referring to fst-name-glyph, those elements also have a more generic classname on them of input-group-addon. And instead of using fst-name-field, we can use form-control.

.input-group .input-group-addon {
    background-color: darkgreen;
    color: yellow;
}
.input-group .form-control {
    background-color: lightgreen;
}
.input-group:hover .input-group-addon {
    width:75px;
    background-color: darkred;
    color: orange;
}
.input-group:hover .form-control {
    background-color: pink;
}

And now, that one single set of CSS declarations works for all of the form fields.

We can now delete all of the mouseenter and mouseleave code, and the CSS declarations cause the form to behave in exactly the same way.

Summary

We took care of the largest amount of duplication by deleting it entirely, replacing it with much more suitable CSS code instead.

Merry Christmas @toronto2009, the worst of the duplication has all been taken care of.

The code as it stands today is found at v0.0.30 in releases

Next time we combine several field value comparisons into the one function.

[Paul_Wilkins]

Now that the worst duplication problem has been taken care of, the remaining sets of duplication should be easy to finish off.

Removing duplication from field value comparisons is what we’re up to today. We will be doing the following:

• moving compare value duplication into a separate function
• putting that function where other code can access it
• updating change-password to use that compare function too

Duplication in comparing field values

Here is the next set of duplication that jsInspect finds:

Match - 5 instances

./registration3\change-password.js:2,6
const passwordsMatchRule = function (input) {
    const form = input.form;
    const passwordInput = form.elements["Password"];
    return passwordInput.value === input.value;
}

./registration3\registration.js:67,72
const checkDifferentThanFirstname = validate.createValidator(
    function (input) {
        const form = input.form;
        const firstname = form.elements["First Name"];
        return firstname.value !== input.value;
    }, "Password shouldn't match first-name"

./registration3\registration.js:74,79
const checkDifferentThanLastname = validate.createValidator(
    function (input) {
        const form = input.form;
        const lastname = form.elements["Last Name"];
        return lastname.value !== input.value;
    }, "Password shouldn't match last-name"

./registration3\registration.js:81,86
const checkDifferentThanCity = validate.createValidator(
    function (input) {
        const form = input.form;
        const city = form.elements["Your City"];
        return city.value !== input.value;
    }, "Password shouldn't match city name"

./registration3\registration.js:100,105
const checkMatchesPassword = validate.createValidator(
    function (input) {
        const form = input.form;
        const password = form.elements["Password"];
        return password.value === input.value;
    }, "Password doesn't match retyped pwd"

Move duplication into a separate function

The duplication here is checking if two values match. Let’s move that behaviour into a separate function.

        function compareField(form, fieldname, str) {
            const field = form.elements[fieldname];
            return field.value === str;
        }

We can use that function from each of the registration functions that have the duplication:

        const checkDifferentThanFirstname = validate.createValidator(
            function (input) {
                // const form = input.form;
                // const firstname = form.elements["First Name"];
                // return firstname.value !== input.value;
                return !compareField(input.form, "First Name", input.value);
            }, "Password shouldn't match first-name"
        );
        const checkDifferentThanLastname = validate.createValidator(
            function (input) {
                // const form = input.form;
                // const lastname = form.elements["Last Name"];
                // return lastname.value !== input.value;
                return !compareField(input.form, "Last Name", input.value);
            }, "Password shouldn't match last-name"
        );
        const checkDifferentThanCity = validate.createValidator(
            function (input) {
                // const form = input.form;
                // const city = form.elements["Your City"];
                // return city.value !== input.value;
                return !compareField(input.form, "Your City", input.value);
            }, "Password shouldn't match city name"
        );
//...
        const checkMatchesPassword = validate.createValidator(
            function (input) {
                // const form = input.form;
                // const password = form.elements["Password"];
                // return password.value === input.value;
                return compareField(input.form, "Password", input.value);
            }, "Password doesn't match retyped pwd"
        );

Move compareField function out to validate code

There is though also the change-password code that has a comparison too, so we should move that compareField function into the validate code.

That means, adding a test for the new feature being added to the validate code.

validate.test.js

    describe("compares a field", function () {
       it("checks that a field matches a value", function () {
           const form = emailInput.form;
           const fieldname = emailInput.name;
           emailInput.value = "test@example.com";
           const result = validate.fieldMatches(form, fieldname, "test@example.com");
           expect(result).to.equal(true);
       });
       it("checks that a field doesn't match", function () {
           const form = emailInput.form;
           const fieldname = emailInput.name;
           emailInput.value = "test@example.com";
           const result = validate.fieldMatches(form, fieldname, "test@example");
           expect(result).to.equal(false);
       });
    });

and we can move that fieldMatches function into the validate code:

    function fieldMatches(form, fieldname, str) {
        const field = form.elements[fieldname];
        return field.value === str;
    }
//...
    return {
        createValidator,
        check,
        fieldMatches,

The registration code can use that validate.fieldMatches function now:

        // function fieldMatches(form, fieldname, str) {
        //     const field = form.elements[fieldname];
        //     return field.value === str;
        // }
        const checkDifferentThanFirstname = validate.createValidator(
            function (input) {
                // return !fieldMatches(input.form, "First Name", input.value);
                return !validate.fieldMatches(input.form, "First Name", input.value);
            }, "Password shouldn't match first-name"
        );
        const checkDifferentThanLastname = validate.createValidator(
            function (input) {
                // return !fieldMatches(input.form, "Last Name", input.value);
                return !validate.fieldMatches(input.form, "Last Name", input.value);
            }, "Password shouldn't match last-name"
        );
        const checkDifferentThanCity = validate.createValidator(
            function (input) {
                // return !fieldMatches(input.form, "Your City", input.value);
                return !validate.fieldMatches(input.form, "Your City", input.value);
            }, "Password shouldn't match city name"
        );
//...
        const checkMatchesPassword = validate.createValidator(
            function (input) {
                // return fieldMatches(input.form, "Password", input.value);
                return validate.fieldMatches(input.form, "Password", input.value);
            }, "Password doesn't match retyped pwd"
        );

Have change-password use fieldMatches function too

The important reason for moving the function into the validate code, is so that the change-password code can use the fieldMatches function too.

change-password.js

    const passwordsMatchRule = function (input) {
        // const form = input.form;
        // const passwordInput = form.elements["Password"];
        // return passwordInput.value === input.value;
        return validate.fieldMatches(input.form, "Password", input.value);
    }

Summary

We moved compare value duplication into a separate function, put that function where other code can access it, and updated change-password to use that compare function too.

The code as it stands today is found at v0.0.31 in releases

Next time we remove duplication from regular expressions.

[Paul_Wilkins]

Removing duplication from regular expression validations is what we’re up to today. We will be doing the following:

• investigating the regular expression duplication
• using a separate checkRx function
• creating a validate test for that checkRx function
• moving the checkRx function into the validate code
• updating registration code to use validate.checkRx

Duplication found in the regular expression validations

Then next set of duplication is found in the regular expressions used for checking the form fields.

Match - 5 instances

./registration3\registration.js:40,48
        return /^([a-zA-Z]{1,})+$/.test(input.value);
    }, "Please enter upper case and lower case only"
);
const checkIsPhoneNumber = validate.createValidator(
    function (input) {
        var phoneReg = /^\(?([0-9]{4})\)?([ .-]?)([0-9]{3})\2([0-9]{4})$/;
        return phoneReg.test(input.value);
    }, "Please enter Phone Number correctly"
);

./registration3\registration.js:46,54
        return phoneReg.test(input.value);
    }, "Please enter Phone Number correctly"
);
const checkIsEmail = validate.createValidator(
    function (input) {
        var emailReg = /^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/;
        return emailReg.test(input.value);
    }, "Please enter it correctly"
);

./registration3\registration.js:52,60
        return emailReg.test(input.value);
    }, "Please enter it correctly"
);
const checkPostalAddress = validate.createValidator(
    function (input) {
        const addressReg = /^\d+\s[A-z]+\s[A-z]+/g;
        return addressReg.test(input.value);
    }, "Please enter Address correctly"
);

./registration3\registration.js:58,66
        return addressReg.test(input.value);
    }, "Please enter Address correctly"
);
const checkPostcode = validate.createValidator(
    function (input) {
        const postcodeReg = /^[a-zA-Z]{1,2}([0-9]{1,2}|[0-9][a-zA-Z])\s*[0-9][a-zA-Z]{2}$/;
        return postcodeReg.test(input.value);
    }, "Please enter Post-code correctly"
);

./registration3\registration.js:79,87
        return !validate.fieldMatches(input.form, "Your City", input.value);
    }, "Password shouldn't match city name"
);
const checkPasswordAtLeastSix = validate.createValidator(
    function (input) {
        var pswReglow = /^([a-zA-Z0-9]{6,})+$/;
        return pswReglow.test(input.value);
    }, "Please enter at least 6 characters"
);

We can remove that duplication by using a separate function to perform the regular expression checks.

Use a separate function for the common behaviour

        function checkRx(rx, input) {
            return rx.test(input.value);
        }
        const checkMoreThanOneAlpha = validate.createValidator(
            function (input) {
                // return !/^([a-zA-Z]{1})$/.test(input.value);
                return !checkRx(/^([a-zA-Z]{1})$/, input);
            }, "Please enter 2 upper case or lower case at least"
        );
        const checkOnlyAlphaChars = validate.createValidator(
            function (input) {
                // return /^([a-zA-Z]{1,})+$/.test(input.value);
                return checkRx(/^([a-zA-Z]{1,})+$/, input);
            }, "Please enter upper case and lower case only"
        );
        const checkIsPhoneNumber = validate.createValidator(
            function (input) {
                // var phoneReg = /^\(?([0-9]{4})\)?([ .-]?)([0-9]{3})\2([0-9]{4})$/;
                // return phoneReg.test(input.value);
                return checkRx(/^\(?([0-9]{4})\)?([ .-]?)([0-9]{3})\2([0-9]{4})$/, input);
            }, "Please enter Phone Number correctly"
        );
        const checkIsEmail = validate.createValidator(
            function (input) {
                // var emailReg = /^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/;
                // return emailReg.test(input.value);
                return checkRx(/^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/, input);
            }, "Please enter it correctly"
        );
        const checkPostalAddress = validate.createValidator(
            function (input) {
                // const addressReg = /^\d+\s[A-z]+\s[A-z]+/g;
                // return addressReg.test(input.value);
                return checkRx(/^\d+\s[A-z]+\s[A-z]+/g, input);
            }, "Please enter Address correctly"
        );
        const checkPostcode = validate.createValidator(
            function (input) {
                // const postcodeReg = /^[a-zA-Z]{1,2}([0-9]{1,2}|[0-9][a-zA-Z])\s*[0-9][a-zA-Z]{2}$/;
                // return postcodeReg.test(input.value);
                return checkRx(/^[a-zA-Z]{1,2}([0-9]{1,2}|[0-9][a-zA-Z])\s*[0-9][a-zA-Z]{2}$/, input);
            }, "Please enter Post-code correctly"
        );

Now that we have a working solution in the way of the checkRx function, we can move it to where it belongs, in the validate code.

Add the checkRx function into the validate code

We first create a validate test, for what the function is going to do:

validate.test.js

    describe("compares against a regular expression", function () {
       it("checks that a field matches a regex", function () {
           emailInput.value = "email@example.com";
           const regex = /[a-z]@[a-z.]/;
           const result = validate.checkRx(regex, emailInput);
           expect(result).to.equal(true);
       });
       it("checks that a field doesn't match regex", function () {
           emailInput.value = "";
           const regex = /[a-z]@[a-z.]/;
           const result = validate.checkRx(regex, emailInput);
           expect(result).to.equal(false);
       });
    });

We can now add the checkRx function to the validate code:

    function checkRx(rx, input) {
        return rx.test(input.value);
    }
//...
    return {
        createValidator,
        check,
        checkRx,

Use the validate.checkRx() function instead

That lets us remove the function from the registration code, and use the validate.checkRx one instead:

        // function checkRx(rx, input) {
        //     return rx.test(input.value);
        // }
        const checkMoreThanOneAlpha = validate.createValidator(
            function (input) {
                // return !checkRx(/^([a-zA-Z]{1})$/, input);
                return !validate.checkRx(/^([a-zA-Z]{1})$/, input);
            }, "Please enter 2 upper case or lower case at least"
        );
        const checkOnlyAlphaChars = validate.createValidator(
            function (input) {
                // return checkRx(/^([a-zA-Z]{1,})+$/, input);
                return validate.checkRx(/^([a-zA-Z]{1,})+$/, input);
            }, "Please enter upper case and lower case only"
        );
        const checkIsPhoneNumber = validate.createValidator(
            function (input) {
                // return checkRx(/^\(?([0-9]{4})\)?([ .-]?)([0-9]{3})\2([0-9]{4})$/, input);
                return validate.checkRx(/^\(?([0-9]{4})\)?([ .-]?)([0-9]{3})\2([0-9]{4})$/, input);
            }, "Please enter Phone Number correctly"
        );
        const checkIsEmail = validate.createValidator(
            function (input) {
                // return checkRx(/^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/, input);
                return validate.checkRx(/^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/, input);
            }, "Please enter it correctly"
        );
        const checkPostalAddress = validate.createValidator(
            function (input) {
                // return checkRx(/^\d+\s[A-z]+\s[A-z]+/g, input);
                return validate.checkRx(/^\d+\s[A-z]+\s[A-z]+/g, input);
            }, "Please enter Address correctly"
        );
        const checkPostcode = validate.createValidator(
            function (input) {
                // return checkRx(/^[a-zA-Z]{1,2}([0-9]{1,2}|[0-9][a-zA-Z])\s*[0-9][a-zA-Z]{2}$/, input);
                return validate.checkRx(/^[a-zA-Z]{1,2}([0-9]{1,2}|[0-9][a-zA-Z])\s*[0-9][a-zA-Z]{2}$/, input);
            }, "Please enter Post-code correctly"
        );

Summary

We investigated the regular expression duplication, used a separate checkRx function, created a validate test for that checkRx function, and moved the checkRx function into the validate code.

The code as it stands today is found at v0.0.32 in releases

Next time we’ll be removing duplication from when setting the error field status.

[Paul_Wilkins]

Removing duplication from when setting error fields to be warning or ok, is what we’re up to today. We will be doing the following:

• investigating duplication in the inputStatus code
• using an errorStatus object to store configuration
• adding a function to use that errorStatus info
• removing the duplication that was found
• cleaning up some of the code

Duplication found in inputStatus code

The next set of duplication that is found is when setting the error status.

Match - 3 instances

./registration3\input-status.js:2,6
function setNone($el, classes) {
    $el.removeClass("warning");
    $el.removeClass("ok");
    $el.removeClass(classes);
}

./registration3\input-status.js:7,11
function setOk($el, classes) {
    $el.removeClass("warning");
    $el.addClass("ok");
    $el.addClass(classes);
}

./registration3\input-status.js:12,16
function setWarning($el, classes) {
    $el.removeClass("ok");
    $el.addClass("warning");
    $el.addClass(classes);
}

Investigating the duplication

I don’t think that this next set of duplication is going to be as easily dealt with by moving code into a separate function.

[checking function, please standby . . .]

No it’s not that easily solved. A suitable solution instead is to use a data object to keep track of what gets shown.

const errorStates = {
    none: {
        remove: "warning ok"
    },
    ok: {
        add: "ok",
        remove: "warning"
    },
    warning: {
        add: "warning",
        remove: "ok"
    }
};

Using the errorStates data

We can now uses that errorStates object from the code:

    function setErrorState($error, state) {
        const errorState = errorStates[state];
        $error.removeClass(errorState.remove);
        $error.addClass(errorState.add);
    }
    function setNone($el, classes) {
        // $el.removeClass("warning");
        // $el.removeClass("ok");
        setErrorState($el, "none");
        $el.removeClass(classes);
    }
    function setOk($el, classes) {
        // $el.removeClass("warning");
        // $el.addClass("ok");
        setErrorState($el, "ok");
        $el.addClass(classes);
    }
    function setWarning($el, classes) {
        // $el.removeClass("ok");
        // $el.addClass("warning");
        setErrorState($el, "warning");
        $el.addClass(classes);
    }

That was an easy removal of duplication, so let’s get some cleaning up done.

Cleaning up the code

The code can be run through beautifier.io for some initial cleanup, and through JSLint for further touches.

Avoid using the this keyword

JSLint warns us about using the this keyword. There is almost always a better way to achieve that code.

The resetForm function can be updated to use the forEach method instead, letting us easily use a function parameter of formGroup instead of the this keyword.

    function resetForm($form) {
       const formGroups = $form.find(".form-group").toArray();
        // $form.find(".form-group").each(function() {
        formGroups.forEach(function resetGroup(formGroup) {
            // var inputName = $(this).find(".check").attr("name");
            var inputName = $(formGroup).find(".check").attr("name");
            // inputStatus.errorOk(this, "Your " + inputName);
            inputStatus.errorOk(formGroup, "Your " + inputName);
            // inputStatus.feedbackNone(this);
            inputStatus.feedbackNone(formGroup);
        });
    }

A benefit of removing the this keyword is that it’s easier to understand that it is the formGroup being used in this function.

Avoid anonymous functions

The only other improvement to the inputStatus function is naming the anonymous function at the start. As it’s an IIFE function I like to start its name with make.

// const inputStatus = (function () {
const inputStatus = (function makeInputStatus() {

Use a function when it’s more appropriate

In the change-password code, we should update the passwordsMatchRule to be a function:

    // const passwordsMatchRule = function(input) {
    function passwordsMatchRule(input) {
        return validate.fieldMatches(input.form, "Password", input.value);
    }

Some tests also need to properly pass the event object to event handlers:

    function passwordInputHandler() {
        const passwordHandler = changePassword.eventHandler.passwordInput;
        const thisArg = $emailGroup.get(0);
        const evt = {target: thisArg};
        // passwordHandler.call(thisArg);
        passwordHandler.call(thisArg, evt);
    }

Update the submit handler

The rest of the change-password code issues can be dealt with by replacing the submit event handler with a copy of what we did on the login page:

    function passwordSubmitHandler(evt) {
        validate.checkFormEmpty("#changepw");
        if ($("#changepw .warning").length !== 0) {
            evt.preventDefault();
        }
    }

And that means, updating the error messages in the tests so that they fit the more standard model of messages that we’ve been using.

    it("email has value", function () {
        //...
        // expect($emailError.html()).to.equal("Your E-mail is OK");
        expect($emailError.html()).to.equal("E-mail is Ok: Your data has been entered correctly");
    });

Preparing for more cleaning

Cleaning up is something that we should do whenever we touch the code. I’ve been a bit lax on it as I’ve been focusing more heavily on removing duplication, but it’s something that I aim to improve on as we do some final work on the code.

To help encourage this cleaning up to occur, I’m leaving a TODO notice at the top of each script file.

// TODO: Clean up using JSLint

I won’t commit this message to the code repository though. That way it remains in my local files as a reminder.

It’s now possible to do the cleanup all at the same time, but as the duplication has us going to different files, I can also take care of it when I work on each individual file.

Summary

Today we used an errorStatus object to store configuration data, letting us remove duplication in the inputStatus code. We also used BeautifyJS and JSLint to clean up some of the code.

The code as it stands today is found at v0.0.33 in releases

Next time we take care of more duplication between the firstname, lastname, and city registration code.

[Paul_Wilkins]

Removing duplication from some of the registration validations is what we’re up to today. We will be doing the following:

• making the regex and checkfield validators similar
• using a data object to store different types of validations

The next set of duplication is found in the firstname, lastname, and city sections that we dealt with last time.

Match - 3 instances

./registration3\registration.js:60,67
        return validate.checkRx(/^[a-zA-Z]{1,2}([0-9]{1,2}|[0-9][a-zA-Z])\s*[0-9][a-zA-Z]{2}$/, input);
    }, "Please enter Post-code correctly"
);
const checkDifferentThanFirstname = validate.createValidator(
    function(input) {
        return !validate.fieldMatches(input.form, "First Name", input.value);
    }, "Password shouldn't match first-name"
);

./registration3\registration.js:65,72
        return !validate.fieldMatches(input.form, "First Name", input.value);
    }, "Password shouldn't match first-name"
);
const checkDifferentThanLastname = validate.createValidator(
    function(input) {
        return !validate.fieldMatches(input.form, "Last Name", input.value);
    }, "Password shouldn't match last-name"
);

./registration3\registration.js:70,77
        return !validate.fieldMatches(input.form, "Last Name", input.value);
    }, "Password shouldn't match last-name"
);
const checkDifferentThanCity = validate.createValidator(
    function(input) {
        return !validate.fieldMatches(input.form, "Your City", input.value);
    }, "Password shouldn't match city name"
);

We removed some parts of the duplication, but another type of duplication remain.

There’s two main types of code in this area, regex code and fieldMatches code.

        const checkPostcode = validate.createValidator(
            function(input) {
                return validate.checkRx(/^[a-zA-Z]{1,2}([0-9]{1,2}|[0-9][a-zA-Z])\s*[0-9][a-zA-Z]{2}$/, input);
            }, "Please enter Post-code correctly"
        );
        const checkDifferentThanFirstname = validate.createValidator(
            function(input) {
                return !validate.fieldMatches(input.form, "First Name", input.value);
            }, "Password shouldn't match first-name"
        );

By making the code similar, it will be easier to remove the duplication between them. We can do that by changing the fieldMatches parameters to give the field name, and the input element.

Waht follows is a two-step process:

1. Make the function parameters for checkRx and fieldMatches more similar to each other
2. Use a data object to call either of those functions with suitable values.

Create an updated fieldMatch function

To help keep things simple, I’ll use a similar-named function of fieldMatch for the updated parameters:

Here’s the test for what we want:

    describe("compares a field", function () {
        const form = $("#changepw").get(0);
        const passwordField = form.elements.Password;
        const retypeField = form.elements["Password Retype"];
       it("checks that a field matches a value", function () {
           passwordField.value = "test password";
           retypeField.value = "test password";
           const result = validate.fieldMatch("Password", retypeField);
           expect(result).to.equal(true);
       });
       it("checks that a field doesn't match", function () {
           passwordField.value = "test password";
           retypeField.value = "bad password";
           const result = validate.fieldMatch("Password", retypeField);
           expect(result).to.equal(false);
       });
    });

The fieldMatch function can be really simple, just passing the right information to the fieldMatches function instead:

validate.js

    function fieldMatch(fieldname, input) {
        return fieldMatches(input.form, fieldname, input.value);
    }
//...
    return {
        //...
        fieldMatches,
        fieldMatch,

and now that things are tested and working correctly, we can update the fieldMatch function so that it doesn’t need to call the fieldMatches one anymore.

    function fieldMatch(fieldname, input) {
        const form = input.form;
        const field = form.elements[fieldname];
        return field.value === input.value;
    }

We can now change the fieldMatches parts of the code to fieldMatch, with the updated function arguments. For example:

        const checkDifferentThanFirstname = validate.createValidator(
            function(input) {
                return // !validate.fieldMatches(input.form, "First Name", input.value);
                return !validate.fieldMatch("First Name", input);
            }, "Password shouldn't match first-name"
        );

which lets us now replace all of the fieldMatches references:

    function passwordsMatchRule(input) {
        // return validate.fieldMatches(input.form, "Password", input.value);
        return validate.fieldMatch("Password", input);
    }

Use a data object to call checkRx and fieldMatch

Now that a similar set of function parameters are being used, we can use a data object to store their unique aspects:

        const validators = {
            lessThanTwentyChars: {
                regex: /^.{1,19}$/,
                error: "Please enter no more than 19 char"
            },
            moreThanOneAlpha: {
                regex: /^([a-zA-Z]{1})$/,
                error: "Please enter 2 upper case or lower case at least"
            },
            onlyAlphaChars: {
                regex: /^([a-zA-Z]{1,})+$/,
                error: "Please enter upper case and lower case only"
            },
            isPhoneNumber: {
                regex: /^\(?([0-9]{4})\)?([ .-]?)([0-9]{3})\2([0-9]{4})$/,
                error: "Please enter Phone Number correctly"
            },
            isEmail: {
                regex: /^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/,
                error: "Please enter it correctly"
            },
            postalAddress: {
                regex: /^\d+\s[A-z]+\s[A-z]+/g,
                error: "Please enter Address correctly"
            },
            postcode: {
                regex: /^[a-zA-Z]{1,2}([0-9]{1,2}|[0-9][a-zA-Z])\s*[0-9][a-zA-Z]{2}$/,
                error: "Please enter Post-code correctly"
            },
            differentThanFirstname: {
                fieldname: "First Name",
                error: "Password shouldn't match first-name"
            },
            differentThanLastname: {
                fieldname: "Last Name",
                error: "Password shouldn't match last-name"
            },
            differentThanCity: {
                fieldname: "Your City",
                error: "Password shouldn't match city name"
            },
            passwordAtLeastSix: {
                regex: /^([a-zA-Z0-9]{6,})+$/,
                error: "Please enter at least 6 characters"
            },
            passwordBelowThirteen: {
                regex: /^([a-zA-Z0-9]{13,})+$/,
                error: "Please enter no more than 12 characters"
            },
            matchesPassword: {
                fieldname: "Password",
                error: "Password doesn't match retyped pwd"
            }
        };

My objective here is to use only the key name, to build a suitable validator function.

        const checkLessThanTwentyChars = validate.createValidator(
            function(input) {
                return validate.checkRx(validators["lessThanTwentyChars"].regex, input);
            }, validators["lessThanTwentyChars"].error
        );

which means that I should be able to put together a separate createValidator function, that accepts only “lessThanTwentyChars” and builds the rest for us.

        function createValidator(validatorName) {
            const fn = function regexValidator(input) {
                return validate.checkRx(validators[validatorName].regex, input);
            };
            const errorMessage = validators[validatorName].error;
            return validate.createValidator(fn, errorMessage);
        }
        const checkLessThanTwentyChars = createValidator("lessThanTwentyChars");

And it works.

Removing duplication tends to mean using a function for similar code, and beyond that moving information into a data structure.

Cleaning up the code

At the time of this writing I have already committed the code changes up to version 37. Because of that, I will be revisiting cleaning up the code from the end of part 37.

Summary

We made the regex and checkfield validators similar, so that we can use a data object to store different types of validations together.

The code as it stands today is found at v0.0.34 in releases

Before working on the other validations, we do need each validation to be consistent. That means not using !validate.checkRx in some places and validate.checkRx in other places. They all need to be the same which means refining the validation regular expressions, which is what the next post is going to be about.

[Paul_Wilkins]

Today’s task is updating the validation code so that it doesn’t invert the result of what it checks, making it easier for us to remove duplication in that area. We will be doing the following:

• removing the need for inverting regular expression results
• replacing code with calls to createValidator instead
• letting createValidator handle both regex and fieldname rules

Using consistency to reveal duplication

Invert the regular expression

The checkMoreThanOneAlpha regular expression currently inverts the result of the validate check. That impeads our ability to give it the same structure as other validators, with the intention to remove that duplication.

Earlier we said that we would have to make the tests consistent, and now is when that occurs.

In our attempt to invert the regular expression, telling the regex just to check for 2 or more characters isn’t successful.

We can update the code to help is reveal the problem:

        const checkMoreThanOneAlpha = validate.createValidator(
            function(input) {
                const value = input.value;
                const expectedRx = !validate.checkRx(/^([a-zA-Z]{1})$/, input);
                const updatedRx = validate.checkRx(/^([a-zA-Z]{2,})$/, input);
                console.log({value, expectedRx, updatedRx});
                return !validate.checkRx(/^([a-zA-Z]{1})$/, input);
            }, "Please enter 2 upper case or lower case at least"
        );

We are shown:

  registration-input first name
     ✓ is empty
     ✓ is fake
     ✓ is too long
 {value: "a", expectedRx: false, updatedRx: false}
     ✓ has too short
 {value: "##", expectedRx: true, updatedRx: false}
     ✓ has strange characters
 {value: "John", expectedRx: true, updatedRx: true}
     ✓ is valid

The strange characters test fails, indicating that the test named checkMoreThanOneAlpha really isn’t checking for more than one alpha, but just more than one character. After all, we have a separate check called checkOnlyAlphaChars that takes care of alphas.

As a result, we can just use a simpler regex of /^.{2,}$/ instead:

  registration-input first name
     ✓ is empty
     ✓ is fake
     ✓ is too long
 {value: "a", expectedRx: false, updatedRx: false}
     ✓ has too short
 {value: "##", expectedRx: true, updatedRx: true}
     ✓ has strange characters
 {value: "John", expectedRx: true, updatedRx: true}
     ✓ is valid

and use that regex in the function:

        const checkMoreThanOneAlpha = validate.createValidator(
            function(input) {
                const value = input.value;
                return validate.checkRx(/^(.{2,}$/, input);
            }, "Please enter 2 upper case or lower case at least"
        );

Use a more appropriate name

The checkMoreThanOneAlpha name is also seen to be slightly misleading, so we can rename it to char instad of alpha too.

        const validators = {
            //...
            // moreThanOneAlpha: {
            moreThanOneChar: {
                // regex: /^([a-zA-Z]{1})$/,
                regex: /^.{1}$/,
                error: "Please enter 2 upper case or lower case at least"
            },
//...
        // const checkMoreThanOneAlpha = validate.createValidator(
        const checkMoreThanOneChar = validate.createValidator(
            function(input) {
                const value = input.value;
                return validate.checkRx(/^(.{2,}$/, input);
            }, "Please enter 2 upper case or lower case at least"
        );
//...
       return validate.check($formGroup, {
            "First Name": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                checkLessThanTwentyChars,
                // checkMoreThanOneAlpha,
                checkMoreThanOneChar,
                checkOnlyAlphaChars
            ],
            "Last Name": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                checkLessThanTwentyChars,
                // checkMoreThanOneAlpha,
                checkMoreThanOneChar,
                checkOnlyAlphaChars
            ],

Replace code with createValidator function call

The code can now be replaced with a single line that creates the validator:

        const checkLessThanTwentyChars = createValidator("lessThanTwentyChars");
        // const checkMoreThanOneChar = validate.createValidator(
        //     function(input) {
        //         const value = input.value;
        //         return validate.checkRx(/^.{2,}$/, input);
        //     }, "Please enter 2 upper case or lower case at least"
        // );
        const checkMoreThanOneChar = createValidator("moreThanOneChar");

and moved down to where checkMoreThanOneChar is used.

        // const checkLessThanTwentyChars = createValidator("lessThanTwentyChars");
        // const checkMoreThanOneChar = createValidator("moreThanOneChar");
//...
        return validate.check($formGroup, {
            "First Name": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                // checkLessThanTwentyChars,
                createValidator("lessThanTwentyChars"),
                // checkMoreThanOneChar,
                createValidator("moreThanOneChar"),
                checkOnlyAlphaChars
            ],
            "Last Name": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                // checkLessThanTwentyChars,
                createValidator("lessThanTwentyChars"),
                // checkMoreThanOneChar,
                createValidator("moreThanOneChar"),
                checkOnlyAlphaChars
            ],

We will have finished this part of the work when all of the sections from firstname through to passwords have been updated to use createValidator instead of variables.

Use createValidator in more places

Most of the remaining sections are easy to convert:

        return validate.check($formGroup, {
            "First Name": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                createValidator("lessThanTwentyChars"),
                createValidator("moreThanOneChar"),
                // checkOnlyAlphaChars
                createValidator("onlyAlphaChars")
            ],
            "Last Name": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                createValidator("lessThanTwentyChars"),
                createValidator("moreThanOneChar"),
                // checkOnlyAlphaChars
                createValidator("onlyAlphaChars")

            ],
            "Phone Number": [
                validate.fn.checkEmpty,
                // checkIsPhoneNumber
                createValidator("isPhoneNumber")
            ],
            "E-mail": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                // checkIsEmail
                createValidator("isEmail")
            ],
            "Postal Address": [
                validate.fn.checkEmpty,
                // checkPostalAddress
                createValidator("postalAddress")
            ],
            "zip code": [
                validate.fn.checkEmpty,
                // checkPostcode
                createValidator("postcode")
            ],
            "Your City": [
                validate.fn.checkEmpty
            ],
            "Password": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                checkDifferentThanFirstname,
                checkDifferentThanFirstname,
                checkDifferentThanLastname,
                checkDifferentThanCity,
                // checkPasswordAtLeastSix,
                createValidator("passwordAtLeastSix"),
                checkPasswordBelowThirteen
            ],
            "Retype Password": [
                validate.fn.checkEmpty,
                checkMatchesPassword
            ]
        });

Converting the rest of the validations

There are two remaining issues left. Some of the functions don’t use a regular expression but instead compare against a different field, and some of them need to have negative tests inverted to be positive tests.

The password matching retype looks to be where beneficial progress can be made from here.

Remove the invert on below thirteen rule

With the checkPasswordBelowThirteen, we can simplify the regex to be just checking for a certain number of characters:

        // var pswRegheigh = /^([a-zA-Z0-9]{13,})+$/;
        var pswRegheigh = /^[a-zA-Z0-9]{13,}$/;
        return !pswRegheigh.test(input.value);

and then invert the requirement, so that we are checking for anywhere from 1 to 12 characters instead. That lets us remove the exclamation mark inverting the result.

        // var pswRegheigh = /^[a-zA-Z0-9]{13,}$/;
        var pswRegheigh = /^[a-zA-Z0-9]{1,12}$/;
        // return !pswRegheigh.test(input.value);
        return pswRegheigh.test(input.value);

We can now entirely remove that checkPasswordBelowThirteen variable function, and just use the regex instead.

        const checkPasswordBelowThirteen = validate.createValidator(
            function(input) {
                var pswRegheigh = /^[a-zA-Z0-9]{1,12}$/;
                return pswRegheigh.test(input.value);
            }, "Please enter no more than 12 characters"
        );
//...
            passwordBelowThirteen: {
                regex: /^[a-zA-Z0-9]{1,12}$/,
                error: "Please enter no more than 12 characters"
            },
//...
            "Password": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                checkDifferentThanFirstname,
                checkDifferentThanLastname,
                checkDifferentThanCity,
                createValidator("passwordAtLeastSix"),
                // checkPasswordBelowThirteen
                createValidator("passwordBelowThirteen")
            ],

Handle either regex or fieldname rules

In the createValidator function we can check if the config is a regex or if it’s a fieldname.

        function createValidator(validatorName) {
            const validatorConfig = validators[validatorName];
            const errorMessage = validatorConfig.error;
            if (validatorConfig.regex) {
                return validate.createValidator(function regexValidator(input) {
                    return validate.checkRx(validatorConfig.regex, input);
                }, errorMessage);
            }
            if (validatorConfig.fieldname) {
                //...
            }
        }

When the config is a fieldname, we want to perform a slightly different match instad.

            if (validatorConfig.fieldname) {
                return validate.createValidator(function fieldnameValidator(input) {
                    return validate.fieldMatch(validatorConfig.fieldname, input);
                }, errorMessage);
            }

That way we can use createValidator for the matching password too.

            "Retype Password": [
                validate.fn.checkEmpty,
                createValidator("matchesPassword")
            ]

Next time

The remaining validations are negative ones, where we are checking that one fieldname doesn’t match another field name.

It would be better if we had two different validate methods, one for a match and one for no match, and we’ll get to that in the next post.

Summary

We removed the need for inverting regular expression results, replaced code with calls to createValidator, and allowed createValidator to handle both regex and fieldname types of rules.

The code as it stands today is found at v0.0.35 in releases

Next time we use a createMatcher function to remove even more duplication.

[Paul_Wilkins]

Moving the createValidator function to the validate code is what is what we’re up to today. We will be doing the following:

• creating a createMatcher function
• also creating a createNomatcher function
• using them to remove duplication
• replacing a flag with a config state

Creating a positive and negative matcher

We have the retype password that needs to match a form value, and the password field must not match other fields such as firstname and city. Those are two conflicting requirements, so it makes sense to to create two different validate functions. A matcher for when it needs to match the requirement, and another one for when it shouldn’t match the requirement.

Using validate.createMatcher and validate.createNoMatcher seems to be a suitable design. This is where we use tests to define requirements for the desired createMatcher and createNoMatcher behaviour.

Create the matcher

I want to make a createMatcher function, that returns either true or false depending on whether it matches a given rule.

Here is the test for that createMatcher behaviour:

validate.test.js

    describe("creates a validator", function () {
        const abcConfig = {
            regex: /^[abc]/,
            error: "Must start with a, b, or c"
        };
        it("can match regex with matcher", function () {
            const abcValidator = validate.createMatcher(abcConfig);
            emailInput.value = "atest@example.com";
            expect(abcValidator(emailGroup)).to.equal(true);
            
        });
        it("doesn't match regex with matcher", function () {
            const abcValidator = validate.createMatcher(abcConfig);
            emailInput.value = "dtest@example.com";
            expect(abcValidator(emailGroup)).to.equal(false);
            
        });
    });

The createValidator function in the registration code is poorly named. We already have a validate.createValidator function that does a different job. So we’ll rename the registration function to be createMatcher, and move it into the validate section of code:

validate.js

    function createMatcher(validatorConfig) {
        const errorMessage = validatorConfig.error;
        if (validatorConfig.regex) {
            return createValidator(function regexValidator(input) {
                return checkRx(validatorConfig.regex, input);
            }, errorMessage);
        }
        if (validatorConfig.fieldname) {
            return createValidator(function fieldnameValidator(input) {
                return fieldMatch(validatorConfig.fieldname, input);
            }, errorMessage);
        }
    }
//...
    return {
        //...
        createValidator,
        createMatcher,

That createMatcher code is getting quite complex though. We can simplify it by extracting the code that deals with whether it is a regex or a fieldname being compared.

validate.js

    function getValidator(config) {
        return config.regex && {
            checker: checkRx,
            rule: config.regex
        }
        || config.fieldname && {
            checker: fieldMatch,
            rule: config.fieldname
        };
    }
    function createMatcher(config) {
        const validator = getValidator(config);
        const errorMessage = config.error;
        return createValidator(function regexValidator(input) {
            return validator.checker(validator.rule, input);
        }, errorMessage);
    }

That code can be optimised further at some later stage. That will happen if we ever happen to deal with more than just the two different situations of regex or fieldname.

Create the non-matcher

For creating a noMatcher method, we add a test for that behaviour:

validate.test.js

        it("doesn't match regex with noMatcher", function () {
            const nonAbcValidator = validate.createNomatcher(abcConfig);
            emailInput.value = "atest@example.com";
            expect(nonAbcValidator(emailGroup)).to.equal(false);
            
        });
        it("matches regex with noMatcher", function () {
            const nonAbcValidator = validate.createNomatcher(abcConfig);
            emailInput.value = "dtest@example.com";
            expect(nonAbcValidator(emailGroup)).to.equal(true);
            
        });

We don’t need much additional code to make that pass. We just invert the validator.checker.

validate.js

    function createNomatcher(config) {
        const validator = getValidator(config);
        const errorMessage = config.error;
        return createValidator(function regexValidator(input) {
            return !validator.checker(validator.rule, input);
        }, errorMessage);
    }
//...
    return {
        //...
        createValidator,
        createMatcher,
        createNomatcher,

But, that has duplication with the createMatcher function. Removing duplication from that can be achieved by passing a flag to change the behaviour of the matcher function.

    // function createMatcher(config) {
    function createMatcher(config, invertMatcher) {
        const validator = getValidator(config);
        const errorMessage = config.error;
        const checker = validator.checker;
        return createValidator(function regexValidator(input) {
            if (invertMatcher) {
                return !checker(validator.rule, input);
            } else {
                return checker(validator.rule, input);
            }
        }, errorMessage);
    }
    function createNomatcher(config) {
        const invertMatcher = true;
        return createMatcher(config, invertMatcher);
    }

That invertMatcher parameter is called a flag. It’s not a good idea to use flags as it can result in too much complexity. It’s only temporary here, until we get things working after which I’ll replace the need for a flag with using a config value instead.

Replacing the createValidator function

Most of the createValidator code can now be replaced, using the createMatcher function:

registration.js

        function createValidator(validatorName) {
        //     const validatorConfig = validators[validatorName];
        //     const errorMessage = validatorConfig.error;
        //     if (validatorConfig.regex) {
        //         return validate.createValidator(function regexValidator(input) {
        //             return validate.checkRx(validatorConfig.regex, input);
        //         }, errorMessage);
        //     }
        //     if (validatorConfig.fieldname) {
        //         return validate.createValidator(function fieldnameValidator(input) {
        //             return validate.fieldMatch(validatorConfig.fieldname, input);
        //         }, errorMessage);
        //     }
            const validatorConfig = validators[validatorName];
            return validate.createMatcher(validatorConfig);
        }

Using createNomatcher

We are now ready to use a createNomatcher function:

As a reminder, here is the duplication in the code that we are trying to get rid of:

registration.js

        const checkDifferentThanFirstname = validate.createValidator(
            function(input) {
                return !validate.fieldMatch("First Name", input);
            }, "Password shouldn't match first-name"
        );
        const checkDifferentThanLastname = validate.createValidator(
            function(input) {
                return !validate.fieldMatch("Last Name", input);
            }, "Password shouldn't match last-name"
        );
        const checkDifferentThanCity = validate.createValidator(
            function(input) {
                return !validate.fieldMatch("Your City", input);
            }, "Password shouldn't match city name"
        );

We can replace their use with the createNomatcher, and remove the unwanted variables:

        // const checkDifferentThanFirstname = validate.createValidator(
        //     function(input) {
        //         return !validate.fieldMatch("First Name", input);
        //     }, "Password shouldn't match first-name"
        // );
        // const checkDifferentThanLastname = validate.createValidator(
        //     function(input) {
        //         return !validate.fieldMatch("Last Name", input);
        //     }, "Password shouldn't match last-name"
        // );
        // const checkDifferentThanCity = validate.createValidator(
        //     function(input) {
        //         return !validate.fieldMatch("Your City", input);
        //     }, "Password shouldn't match city name"
        // );
//...
"Password": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                // checkDifferentThanFirstname,
                createNomatcher("differentThanFirstname"),
                // checkDifferentThanLastname,
                createNomatcher("differentThanLastname"),
                // checkDifferentThanCity,
                createNomatcher("differentThanCity"),
                //...
            ],

Replace flag with config state

Now that things are working, I can remove the need for the createMatcher flag too.

As only a few of the matchers shouldn’t match, we can add a shouldMatch property to the validation data object to modify its behaviour.

            differentThanFirstname: {
                fieldname: "First Name",
                shouldMatch: false,
                error: "Password shouldn't match first-name"
            },
            differentThanLastname: {
                fieldname: "Last Name",
                shouldMatch: false,
                error: "Password shouldn't match last-name"
            },
            differentThanCity: {
                fieldname: "Your City",
                shouldMatch: false,
                error: "Password shouldn't match city name"
            },

That way, the createMatcher function can check if shouldMatch is false, and invert the matcher itself.

We want a test to check that this behaviour works properly, which means updating the validate test:

validate.test.js

        const noAbcConfig = {
            regex: /^[abc]/,
            shouldMatch: false,
            error: "Must start with a, b, or c"
        };
//...
        it("doesn't match regex with noMatcher", function () {
            const nonAbcValidator = validate.createMatcher(noAbcConfig);
            emailInput.value = "atest@example.com";
            expect(nonAbcValidator(emailGroup)).to.equal(false);
            
        });
        it("matches regex with noMatcher", function () {
            const nonAbcValidator = validate.createMatcher(noAbcConfig);
            emailInput.value = "dtest@example.com";
            expect(nonAbcValidator(emailGroup)).to.equal(true);
            
        });

The createMatcher function can be updated to check for shouldMatch:

validate.js

    function createMatcher(config, invertMatcher) {
        const validator = getValidator(config);
        const errorMessage = config.error;
        const checker = validator.checker;
        return createValidator(function regexValidator(input) {
            // if (invertMatcher) {
            if (config.shouldMatch === false) {
                return !checker(validator.rule, input);
            } else if (invertMatcher) {
                return !checker(validator.rule, input);
            } else {
                return checker(validator.rule, input);
            }
        }, errorMessage);
    }

letting us use createMatcher instead of createNomatcher:

registration.js

            "Password": [
                validate.fn.checkEmpty,
                validate.fn.checkFake,
                // createNomatcher("differentThanFirstname"),
                createMatcher("differentThanFirstname"),
                // createNomatcher("differentThanLastname"),
                createMatcher("differentThanLastname"),
                // createNomatcher("differentThanCity"),
                createMatcher("differentThanCity"),
                createMatcher("passwordAtLeastSix"),
                createMatcher("passwordBelowThirteen")
            ],

and the flag can be removed from createMatcher, along with the createNomatcher code:

    // function createMatcher(config, invertMatcher) {
    function createMatcher(config) {
        const validator = getValidator(config);
        const errorMessage = config.error;
        const checker = validator.checker;
        return createValidator(function regexValidator(input) {
            if (config.shouldMatch === false) {
                return !checker(validator.rule, input);
            // } else if (invertMatcher) {
            //     return !checker(validator.rule, input);
            } else {
                return checker(validator.rule, input);
            }
        }, errorMessage);
    }
    // function createNomatcher(config) {
    //     const invertMatcher = true;
    //     return createMatcher(config, invertMatcher);
    // }
//...
    return {
        //...
        createMatcher,
        // createNomatcher,

Summary

We have created a createMatcher function, a createNomatcher function, used them to remove duplication, and replaced a flag with a config state instead.

The code as it stands today is found at v0.0.36 in releases

Next time we remove more duplication from the registration validation.

[Paul_Wilkins]

Today, more duplication is found in the registration validation, and the setup code. We will be doing the following:

• removing duplication from the registration validation
• removing duplication from the setup section of code
• using a linter to help us clean up common code issues

Duplication found in firstname and lastname validator

Here is the duplication that is found:

Match - 3 instances

./registration3\registration.js:97,103
"First Name": [
    validate.fn.checkEmpty,
    validate.fn.checkFake,
    createMatcher("lessThanTwentyChars"),
    createMatcher("moreThanOneChar"),
    createMatcher("onlyAlphaChars")
],

./registration3\registration.js:104,111
"Last Name": [
    validate.fn.checkEmpty,
    validate.fn.checkFake,
    createMatcher("lessThanTwentyChars"),
    createMatcher("moreThanOneChar"),
    createMatcher("onlyAlphaChars")

],

./registration3\registration.js:132,137
"Password": [
    validate.fn.checkEmpty,
    validate.fn.checkFake,
    createMatcher("differentThanFirstname"),
    createMatcher("differentThanLastname"),
    createMatcher("differentThanCity"),

That duplication can be dealt with by using a separate variable for the name validation config.

        const nameValidationConfig = [
            validate.fn.checkEmpty,
            validate.fn.checkFake,
            createMatcher("lessThanTwentyChars"),
            createMatcher("moreThanOneChar"),
            createMatcher("onlyAlphaChars")
        ];
        return validate.check($formGroup, {
        return validate.check($formGroup, {
            // "First Name": [
            //     validate.fn.checkEmpty,
            //     validate.fn.checkFake,
            //     createMatcher("lessThanTwentyChars"),
            //     createMatcher("moreThanOneChar"),
            //     createMatcher("onlyAlphaChars")
            // ],
            "First Name": nameValidationConfig,
            // "Last Name": [
            //     validate.fn.checkEmpty,
            //     validate.fn.checkFake,
            //     createMatcher("lessThanTwentyChars"),
            //     createMatcher("moreThanOneChar"),
            //     createMatcher("onlyAlphaChars")
            // ],
            "Last Name": nameValidationConfig,

The duplications that we are finding now do certainly seem to be less useful now. There are though only 5 sets of duplication remaining, so we’ll keep an eye on that.

Duplication found in setup

The next set of duplication is found in the setup.js code:

Match - 3 instances

./registration3\setup.js:2,5
$("#changepsw").on("click", function() {
    $("#login").css("display", "none");
    $("#changepw").css("display", "block");
});

./registration3\setup.js:6,9
$("#changepsw2").on("click", function() {
    $("#login").css("display", "none");
    $("#changepw").css("display", "block");
});

./registration3\setup.js:10,13
$("#login2").on("click", function() {
    $("#login").css("display", "block");
    $("#changepw").css("display", "none");
});

It should be possible to use two separate functions, one to show the login, and another to show the changePassword form.

But, before making any changes to the code we need to ensure that there are tests that check and ensure that things are working as they should.

setup.test.js

describe("setup", function () {
    after(function () {
        $("#login").css("display", "none");
        $("#changepw").css("display", "none");
    });
    it("when #changepsw clicked, shows change password", function () {
        $("#login").css("display", "block");
        $("#changepw").css("display", "none");
        $("#changepsw").trigger("click");
        expect($("#login").css("display")).to.contain("none");
        expect($("#changepw").css("display")).to.contain("block");
    });
    it("when #changepsw2 clicked, shows change password", function () {
        $("#login").css("display", "block");
        $("#changepw").css("display", "none");
        $("#changepsw2").trigger("click");
        expect($("#login").css("display")).to.contain("none");
        expect($("#changepw").css("display")).to.contain("block");
    });
    it("when #login clicked, shows login", function () {
        $("#login").css("display", "none");
        $("#changepw").css("display", "block");
        $("#login2").trigger("click");
        expect($("#login").css("display")).to.contain("block");
        expect($("#changepw").css("display")).to.contain("none");
    });
});

Now that the tests are in place, we can start to reorganise the setup code.

Moving common code into a single function is the solution here.

login.js

    function showChangePassword() {
        $("#login").css("display", "none");
        $("#changepw").css("display", "block");
    }
    function showLogin() {
        $("#login").css("display", "block");
        $("#changepw").css("display", "none");
    }
    // $("#changepsw").on("click", function() {
    //     $("#login").css("display", "none");
    //     $("#changepw").css("display", "block");
    // });
    $("#changepsw").on("click", showChangePassword);
    // $("#changepsw2").on("click", function() {
    //     $("#login").css("display", "none");
    //     $("#changepw").css("display", "block");
    // });
    $("#changepsw2").on("click", showChangePassword);
    // $("#login2").on("click", function() {
    //     $("#login").css("display", "block");
    //     $("#changepw").css("display", "none");
    // });
    $("#login").on("click", showLogin);
    $("#myBtn").click(function() {
        $("#myModal").modal();
    });

But still, duplication is found between the showChangePassword function and the showLogin function. Does that improve when using jQuery’s show and hide methods?

    function showChangePassword() {
        // $("#login").css("display", "none");
        $("#login").hide();
        // $("#changepw").css("display", "block");
        $("#changepw").show();
    }
    function showLogin() {
        // $("#login").css("display", "block");
        $("#login").show();
        // $("#changepw").css("display", "none");
        $("#changepw").hide();
    }

That duplication is now all taken care of, and there are just four sets of duplication left to go.

Cleaning up the Registration code

Since v0.0.34 we have made significant changes to the registration code, we should do some cleaning up of the code. A good way to approach that is by using JSLint. It’s a harsh mistress, but has good advice.

Regular expressions

The regular expression for the phone number shouldn’t use spaces as that can be unclear, and the hyphen should be escaped too. The \2 also should be replaced with the actual regex that’s wanted there too, which means that the capturing parenthesis aren’t needed on the first set of separators.

            isPhoneNumber: {
                // regex: /^\(?([0-9]{4})\)?([ .-]?)([0-9]{3})\2([0-9]{4})$/,
                regex: /^\(?([0-9]{4})\)?[\u0020.\-]?([0-9]{3})[\u0020.\-]?([0-9]{4})$/,
                error: "Please enter Phone Number correctly"
            },

The email regular expression shouldn’t have the fullstop escaped when it’s in the square brackets of a character class, but we should escape the hyphens.

            isEmail: {
                // regex: /^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/,
                regex: /^([\w\-.]+@([\w\-]+\.)+[\w\-]{2,4})?$/,
                error: "Please enter it correctly"
            },

The this keyword

The remaining things to clean up looks to be about the this keyword and using double quotes instead of single quotes.

It’s not a good idea to use the this keyword. It’s so flexible that in a few lines of code it can refer to several different things. As a result, replacing it with a suitably named variable is a better solution instead.

Here we replace the this keyword with the listItem name instead.

    // function citylistClickHandler() {
    function citylistClickHandler(evt) {
        const listItem = evt.target;
        // const $form = $(this).closest("form");
        const $form = $(listItem).closest("form");
        const $cityField = $form.find("[name='Your City']");
        const $cityGroup = $cityField.closest(".form-group");
        // const value = $(this).text().trim();
        const value = $(listItem).text().trim();
        $cityField.val(value);
        $("#citylist").collapse("hide");

Later on in the registration input handler, we do a similar thing too.

        const inputGroup = evt.target;
        // const $formGroup = $(this).closest(".form-group");
        const $formGroup = $(inputGroup).closest(".form-group");

and with the resetMessages code, the value is passed in to the function:

    // function resetMessages() {
    function resetMessages(i, formGroup) {
        // const $error = $(this).find(".error");
        const $error = $(formGroup).find(".error");
        // const name = $(this).find(".check").attr("name");
        const name = $(formGroup).find(".check").attr("name");
        // inputStatus.errorOk(this, name);
        inputStatus.errorOk(formGroup, name);
        // inputStatus.feedbackNone(this);
        inputStatus.feedbackNone(formGroup);
        // inputStatus.requiredOk(this);
        inputStatus.requiredOk(formGroup);
    }

Use double quotes instead of single quotes

When there’s a choice of quotes to use, and one type of them causes slightly more problems, it’s best to standardise on using only one set of them to avoid conflicts. In this case, the double quotes are the standard.

    // $('.input-group').on('focusin focusout input', registrationInputHandler);
    $(".input-group").on("focusin focusout input", registrationInputHandler);
//...
    // $('#terms').click(termsClickHandler);
    $("#terms").click(termsClickHandler);

Cleanup the setup code

With the setup code, unnamed functions are the main complaint. We can easily deal with one of them because it’s in the document ready code, and we have no need for that document ready part. It can be completely removed.

// $(document).ready(function() {
    function showChangePassword() {
        $("#login").hide();
        $("#changepw").show();
    }
    function showLogin() {
        $("#login").show();
        $("#changepw").hide();
    }
    $("#changepsw").on("click", showChangePassword);
    $("#changepsw2").on("click", showChangePassword);
    $("#login2").on("click", showLogin);

    $("#myBtn").click(function() {
        $("#myModal").modal();
    });
// });

The other unnamed function is the #myBtn click function. Naming that function makes it easier to understand what’s happening there, and we can use the improved on notation that jQuery uses for its event handlers now.

// $("#myBtn").click(function() {
$("#myBtn").on("click", function loginClickHandler() {
    $("#myModal").modal();
});

There is still some more cleaning up to do in there, and we’ll come back to tha later on.

Summary

We removed duplication from the registration validation, and from the setup section, and used JSLint to help tidy up several issues in the code.

The code as it stands today is found at v0.0.37 in releases

Next time we clean up the validate code before seeing to more duplication in there.

[Paul_Wilkins]

Removing duplication from the validate email and password rules is what we’re focused on. We want to clean up the code first though before doing extensive work on it. We will be doing the following:

• Taking care of too-long lines
• Fixing regular expressions
• Dealing with the this keyword
• Making && and || code easier to understand
• Naming anonymous functions

Cleaning up the validate code

Before making further changes to the validate code, we’ll clean things up which should make it easier to work with the code.

Lines too long

Lines shouldn’t be longer than 80 characters. That isn’t only for viewing on smaller screens, but for making it easier for us to understand things more clearly too.

Shorten array list

The list of array items is too long as it is, so putting them into a vertical list is the thing to do.

    const defaultValidators = {
        // "E-mail": [checkEmpty, checkFake, checkEmailReg],
        // "Password": [
        //     checkEmpty, checkFake, checkPasswordShort, checkPasswordLong
        // ]
        "E-mail": [
            checkEmpty,
            checkFake,
            checkEmailReg
        ],
        "Password": [
            checkEmpty,
            checkFake,
            checkPasswordShort,
            checkPasswordLong
        ]
    };

Shorten messages

The inputStatus lines are too long too. We should be replacing those with createMatcher instead, but for now we can split the lines:

        if (getValue(inputGroup) === "") {
            // inputStatus.warning(inputGroup, getName(inputGroup) + " is Empty: Please enter data into this input");
            const msg = "Please enter data into this input";
            inputStatus.warning(
                inputGroup,
                getName(inputGroup) + " is Empty: " + msg
            );
            return false;
        }

Fix regular expressions

There are a number of small issues with the regular expressions. Here’s how they’re resolved.

Escaping characters

Inside of the character group, the fullstop shouldn’t be escaped and the hypen should be.

    function checkEmailReg(inputGroup) {
        // const emailReg = /^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/;
        const emailReg = /^([\w\-.]+@([\w\-]+\.)+[\w\-]{2,4})?$/;
        const value = getValue(inputGroup);

Replace backreferences?

JsLint doesn’t like backreferences.

    function checkFake(inputGroup) {
        const fakeReg = /(.)\1{2,}/;
        const value = getValue(inputGroup);

Using backreferences to find repetition is about the only effective way to do so. I’m not sold on finding repetition though. There are fully valid cases of repetition that wouldn’t be allowed, such as aaaron, and Bess Stewart who’s email address would be bessstewart@example.com.

Because JSLint is against backreferences, and it’s difficult to support the use of finding repetition in names, I have no problem with removing checkFake from the code.

validate.js

    // function checkFake(inputGroup) {
    //     const fakeReg = /(.)\1{2,}/;
    //     const value = getValue(inputGroup);
    //     if (fakeReg.test(value)) {
    //         const msg = "Please remove repetition";
    //         inputStatus.warning(
    //             inputGroup,
    //             getName(inputGroup) + " is Fake text: " + msg
    //         );
    //         return false;
    //     }
    //     return true;
    // }
//...
    const defaultValidators = {
        "E-mail": [
            checkEmpty,
            // checkFake,
            checkEmailReg
        ],
        "Password": [
            checkEmpty,
            // checkFake,
            checkPasswordShort,
            checkPasswordLong
        ]
    };
//...
        fn: {
            getName,
            getValue,
            checkEmpty,
            // checkFake
        }

registration.js

        const nameValidationConfig = [
            validate.fn.checkEmpty,
            // validate.fn.checkFake,
            //...
        ];
//...
            "E-mail": [
                validate.fn.checkEmpty,
                // validate.fn.checkFake,
                createMatcher("isEmail")
            ],
//...
            "Password": [
                validate.fn.checkEmpty,
                // validate.fn.checkFake,
                //...
            ],

Dealing with the this keyword

The this keyword tends to cause trouble when it comes to understanding the code, so it gets replaced with easier to understand code.

        const $formGroups = $(form).find(".form-group").has(".check");
        // $formGroups.each(function validateGroup() {
        $formGroups.each(function validateGroup(i, formGroup) {
            // checkFieldEmpty(this);
            checkFieldEmpty(formGroup);
        });

End a statement with a semicolom

Lastly, we need to add a semicolon to the end of the return statement in this code.

    function createValidator(rule, errorMessage) {
        return function checkInput(inputGroup) {
            //...
        // }
        };
    }

That was easily missed because the function was embedded as a part of the statement itself. Extracting the function help to make things more obvious.

    function createValidator(rule, errorMessage) {
        function checkInput(inputGroup) {
            //...
        }
        return checkInput;
    }

Remove confusion from && and || operators

The last issue that’s shown is about making && and || parts within a statement less confusing to understand. In this case, it is to wrap the && parts in parenthesis to help make it clearer how the code is interpreted.

    function getValidator(config) {
        // return config.regex && {
        return (config.regex && {
            checker: checkRx,
            rule: config.regex
        // }
        // || config.fieldname && {
        }) || (config.fieldname && {
            checker: fieldMatch,
            rule: config.fieldname
        });
    }

That was the last item in the list, but after clearing that up a whole new set of issues are found.

Leave no anonymous function unnamed

The next issue is not exactly about anonymous functions, but it’s related. Cleaning up the code with beautifier.io removes the space between the function keyword and the parameter parenthesis. JSLint notices that lack of space and warns us about it. Those systems act as a handy way to find anonymous functions.

We could add the space back in and JSLint is all good, but this also acts as a handy way to find functions that should be named. In this case, I’ll name it makeValidator.

// const validate = (function() {
const validate = (function makeValidator() {

The other anonymous function is when validating the different types of checks. We can call it checkInput:

        function validateByTypes(inputGroup) {
            //...
            // return types.every(function(checkerFn) {
            return types.every(function checkInput(checkerFn) {
                return checkerFn(inputGroup);
            });
        }

Summary

We reduced the length of too-long lines, fixed regular expressions, dealt with the this keyword, made && and || code easier to understand, and gave names to anonymous functions.

The code as it stands today is found at v0.0.38 in releases

Next time we remove duplication from the rules in the validate code.

[Paul_Wilkins]

Removing duplication from the validate rules is what we’re up to today. We will be doing the following:

• organise functions into related groups
• replacing the changePassword createValidator with createMatcher
• letting the createValidator use the rules config
• replace the validate rules functions with a rules object
• move the showValid function inline

Duplication found in validate email and password rules

Here are two of the 4 sets of duplication found:

Match - 4 instances

./registration3\validate.js:11,23
    if (getValue(inputGroup) === "") {
        const msg = "Please enter data into this input";
        inputStatus.warning(
            inputGroup,
            getName(inputGroup) + " is Empty: " + msg
        );
        return false;
    }
    return true;
}

function checkEmailReg(inputGroup) {
    const emailReg = /^([\w\-.]+@([\w\-]+\.)+[\w\-]{2,4})?$/;

./registration3\validate.js:25,37
    if (!emailReg.test(value)) {
        const msg = "Please enter it correctly";
        inputStatus.warning(
            inputGroup,
            getName(inputGroup) + " is Incorrect: " + msg
        );
        return false;
    }
    return true;
}

function checkPasswordShort(inputGroup) {
    const pswReglow = /^([a-zA-Z0-9]{0,5})$/;

We should be able to remove that duplication by using the createMatcher function that we obtained in the last post. To successfully achieve that, I want the createMatcher and associated functions to be above the rules. That means reorganising some of the functions.

As getName is used by the createValidator function, I’ve moved getName and getValue right to the top. The checkRx and checkMatch functions are used by the createMatcher function, so those have been moved up near the top too.

I end up with three different groups of functions, one set for the matcher, one set for the rules, and one set for doing the checking.

const validate = (function makeValidator() {
    function getName(inputGroup) {
        //...
    }
    function getValue(inputGroup) {
        //...
    }
    function checkRx(rx, input) {
        //...
    }
    function fieldMatch(fieldname, input) {
        //...
    }
    function createValidator(rule, errorMessage) {
        //...
    }
    function getValidator(config) {
        //...
    }
    function createMatcher(config) {
        //...
    }

    function checkEmpty(inputGroup) {
        //...
    }
    function checkEmailReg(inputGroup) {
        //...
    }
    function checkPasswordShort(inputGroup) {
        //...
    }
    function checkPasswordLong(inputGroup) {
        //...
    }
    function showValid(inputGroup) {
        //...
    }

    const defaultValidators = {
        //...
    };
    function check(inputGroup, validators) {
        //...
    }
    function checkFieldEmpty(formGroup) {
        //...
    }
    function checkFormEmpty(form) {
        //...
    }
    return {
        checkRx,
        fieldMatch,
        createValidator,
        createMatcher,
        check,
        checkFieldEmpty,
        checkFormEmpty,
        fn: {
            getName,
            getValue,
            checkEmpty
        }
    };
}());

The plan for today is to replace the checkEmpty, checkEmailReg, checkPasswordShort, checkPasswordLong, and showValid functions with code created by the createMatcher function instead.

Replacing the isEmpty function

Because we want to be consistent in how we use the validators, I want this validator to express what we want to happen. Instead of calling it isEmpty which is the problem we want to avoid, and avoiding negative terms such as notEmpty, I’ll instead call its replacement hasContent so that it tells us what is expected.

validate.js

    const validators = {
        hasContent: {
            regex: /\w/,
            error: "Please enter data into this input"
        }
    };
//...
    const defaultValidators = {
        "E-mail": [
            // isEmpty,
            createMatcher(validators.hasContent),
            checkEmailReg
        ],

The only problem is that a slightly different error message is expected:

      + expected - actual

      -E-mail is Incorrect: Please enter data into this input
      +E-mail is Empty: Please enter data into this input

We could either change the error message expectation to just be for the Incorrect type each time, or we can add information to the config about the type of error. I would rather do the latter.

Let’s add an “Empty” error type to the config:

    const validators = {
        hasContent: {
            regex: /\w/,
            errorType: "Empty",
            error: "Please enter data into this input"
        }
    };

I then need to update createValidator. Currently it receives only the rule and the message, but I want the whole config to be given to it instead. That way it can retrieve what it needs from that config.

Before I can do that, I want the createValidator to be used by as few things as possible. Currently the changePassword still uses createValidator, when it really can be using createMatcher instead. That means updating the changePassword code so that it uses createMatcher instead of createValidator.

To most easily achieve that, I want all of the test results to pass, so it means temporarily renaming hasContent back to checkEmail while removing createValidator from the changePassword code.

    const defaultValidators = {
        "E-mail": [
            // hasContent,
            checkEmpty,
            checkEmailReg
        ],

We now have the following multi-step plan to improve the createValidator function:

1. replace changePassword use of createValidator with createMatcher
2. update createValidator to get info from the validation config
3. have createValidator use the errorType config parameter if it exists

Improve createValidator function

Before we can have the createValidator function use the errorType info, we need to stop other things using createValidator.

1. Replace changePassword createValidator

Replacing createValidator with using createMatcher instead is something that should have been done earlier on, and gets done now.

change-password.js

    // function passwordsMatchRule(input) {
    //     return validate.fieldMatch("Password", input);
    // }
    const retypeValidator = {
        "Password Retype": [
            validate.fn.checkEmpty,
            // validate.createValidator(
            //     passwordsMatchRule,
            //     "Password doesn't match retyped password"
            // )
            validate.createMatcher({
                fieldname: "Password",
                error: "Password doesn't match retyped password"
            })
        ]
    };

Now that nothing else is using createValidator, we can remove it from the list of exports in the validate code:

validate.js

    return {
        checkRx,
        fieldMatch,
        // createValidator,
        createMatcher,

That causes a couple of tests about the custom validator to break, which aren’t needed anymore because the createValidator function is now properly tested using the createMatcher code instead. We can appropriately remove the custom validator tests:

    // describe("uses custom validator", function () {
    //     ...
    // });

We can now move on to part 2, updating the createValidator parameters.

2. Update createValidator parameters

Currently the createValidator function has rule and message parameters. We are also wanting it to have messageType information. One possibility is to pass all three to the createValidator function, but a possibly simpler solution is to just pass all of it as a single data object instead.

While I’m at it, I had better switch the parameters so that the rule function is given as a callback function.

    // function createValidator(rule, errorMessage) {
    function createValidator(ruleConfig) {
        const errorMessage = ruleConfig.error;
//...
            // if (!rule(input)) {
            if (!callback(input)) {
//...
    function createMatcher(config) {
        // ...
        // const errorMessage = config.error;
        // ...
        // return createValidator(function regexValidator(input) {
        //     ...
        // }, errorMessage);
        return createValidator(config, function regexValidator(input) {
            // ...
        });
    }

3. Get the error type from the config object

We can now get the error type from the config, defaulting to using Incorrect if nothing is found.

            if (!callback(input)) {
                // const msg = " is Incorrect: " + errorMessage;
                const type = config.errorType || "Incorrect";
                const heading = getName(inputGroup) + " is " + type + ": ";
                // inputStatus.warning(inputGroup, getName(inputGroup) + msg);
                inputStatus.warning(inputGroup, heading + errorMessage);
                return false;
            }

That’s all of the preparation that’s needed. We can now carry on with replacing the isError function with hasContent using createMatcher instead.

Replace checkEmpty with hasContent

After all of that preparation, it will be easy to replace checkEmpty with hasContent instead.

    const defaultValidators = {
        "E-mail": [
            // checkEmpty,
            createMatcher(validators.hasContent),
            checkEmailReg
        ],

We can now move the other rule functions into a rules object:

Replace email rule function with createMatcher

    // function checkEmailReg(inputGroup) {
    //     const emailReg = /^([\w\-.]+@([\w\-]+\.)+[\w\-]{2,4})?$/;
    //     const value = getValue(inputGroup);
    //     if (!emailReg.test(value)) {
    //         const msg = "Please enter it correctly";
    //         inputStatus.warning(
    //             inputGroup,
    //             getName(inputGroup) + " is Incorrect: " + msg
    //         );
    //         return false;
    //     }
    //     return true;
    // }
    const validators = {
        //...
        isEmail: {
            regex: /^([\w\-.]+@([\w\-]+\.)+[\w\-]{2,4})?$/,
            error: "Please enter it correctly"
        }
    };
    const defaultValidators = {
        "E-mail": [
            createMatcher(validators.hasContent),
            // checkEmailReg,
            createMatcher(validators.isEmail
        ],
        "Password": [
            // checkEmpty,
            createMatcher(validators.hasContent),
            checkPasswordShort,
            checkPasswordLong
        ]
    };

Replace password rule function with createMatcher

The password functions can be replaced with suitable rules too, letting us use createMatcher for them too.

validate.js

    const validators = {
        //...
        passwordAtLeastSix: {
            regex: /^([a-zA-Z0-9]{6,})+$/,
            error: "Please enter at least 6 characters"
        },
        passwordBelowThirteen: {
            regex: /^[a-zA-Z0-9]{1,12}$/,
            error: "Please enter no more than 12 characters"
        }
    };
    // function checkPasswordShort(inputGroup) {
    //     ...
    // }
    // function checkPasswordLong(inputGroup) {
    //     ...
    // }
//...
    const defaultValidators = {
        "E-mail": [
            createMatcher(validators.hasContent),
            createMatcher(validators.isEmail)
        ],
        "Password": [
            createMatcher(validators.hasContent),
            // checkPasswordShort,
            createMatcher(validators.passwordAtLeastSix),
            // checkPasswordLong,
            createMatcher(validators.passwordBelowThirteen)
        ]
    };

Summary

We organised functions into related groups, replaced the changePassword createValidator with createMatcher letting us have the createValidator use the rules config, and replaced the validate rules functions with a rules object.

The code as it stands today is found at v0.0.39 in releases

Next time we remove the other password validation rules from elsewhere, and replace checkEmpty with hasContent throughout all of the code.