3
\$\begingroup\$

I'm using the following piece of JavaScript to emulate class extending. Is this a valid way to go or does it have some drawbacks which should definitely be fixed?

var Validator = function (inputId) {
    "use strict";

    var self = this;

    self.inputId = inputId;

    window.document.getElementById(inputId).onblur = function () {
        self.check();
    };

};

Validator.prototype.inputId = '';

/**
 * Validator fails by default
 *  
 * @returns {Boolean} 
 */
Validator.prototype.isValid = function () {
    "use strict";

    return false;

};

/**
 * Check whether the input is valid
 * 
 * @returns {Validator}
 */
Validator.prototype.check = function () {
    "use strict";

    // Check and mark whether validation passes
    if (!this.isValid()) {

        this.markInvalid();

    } else {

        this.markValid();

    }

    return this;

};

/**
 * Mark the input as valid
 * 
 * @returns {Validator}
 */
Validator.prototype.markValid = function () {
    "use strict";

    var input = window.document.getElementById(this.inputId);

    // Remove "error" class name
    input.className = input.className.replace(
        /(?:^|\s)error(?!\S)/g,
        ''
    );

    return this;

};

/**
 * Mark input as invalid
 * 
 * @returns {Validator}
 */
Validator.prototype.markInvalid = function () {
    "use strict";

    var input = window.document.getElementById(this.inputId);

    // Add "error" class when it is not set already
    if (!input.className.match(/(?:^|\s)error(?!\S)/g)) {
        input.className += ' error';
    }

    return this;

};

Than this is used as a concrete validator implementation

// Inherit from validator
var VatCheck = Validator;

/**
 * Validate vat number
 * 
 * @note only format is checked, not whether the number is in use
 * 
 * @returns {Boolean} 
 */
VatCheck.prototype.isValid = function () {
    "use strict";

    var input = window.document.getElementById(this.inputId);

    // Check number by using Vat.js
    return !!checkVATNumber(input.value);

};

The reason I came to this solution is because only one or two methods of the parent class have different behavior, and the use of the strategy pattern seems somewhat over-engineering

\$\endgroup\$
7
  • \$\begingroup\$ This looks like example code... are you just toying around or are you planning an implementation of this? \$\endgroup\$
    – Pimgd
    Jan 30, 2015 at 10:40
  • \$\begingroup\$ @Pimgd I'm working on an implementation of a form validator. Just removed the function implementation to be able to review the structure and make the example clearer. Code reviewing a technique or structure does not need to be production code is it? \$\endgroup\$ Jan 30, 2015 at 10:47
  • 1
    \$\begingroup\$ Sorry, real code only. Right now, we cannot provide you with a contextual answer. That means we're limited to the bits of code you've provided - for which the solution would probably work well enough. Then you use that everywhere where you need inheritance and due to some oversight, you've just created a maintenance nightmare. \$\endgroup\$
    – Pimgd
    Jan 30, 2015 at 10:48
  • \$\begingroup\$ It's not a bad question, it's just that right now, there's a high chance of you getting inappropriate advice which you will use to shoot yourself in the foot. \$\endgroup\$
    – Pimgd
    Jan 30, 2015 at 10:54
  • 2
    \$\begingroup\$ What you may and may not do after receiving answers - in this case, I suggest taking your code and sorting out the issues that have been pointed out, and then re-posting it as a new question here on Code Review. \$\endgroup\$
    – rolfl
    Jan 30, 2015 at 16:47

1 Answer 1

2
\$\begingroup\$

There is one bad problem in the code that can be answered by the basis of the pattern presented here:

// Inherit from validator
var VatCheck = Validator;

This does not inherit anything, instead it causes VatCheck name to point to the exact same object (constructor function) as the Validator. Thus all you are doing is overwriting Validator.prototype.isValid (changing the base implementation), and aliasing the base implementation by different names.

If you repeat this pattern again, the end result is that all validators have their isValid check method do the action of the last validator type that you define.

Instead you need to do VatCheck.prototype = new Validator() but then you need to also redo the constructor.

\$\endgroup\$
1
  • \$\begingroup\$ Ok, makes a lot of sense. I'll modify the inheriting class and move the constructor logic to a separate initialization function. \$\endgroup\$ Jan 30, 2015 at 14:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.