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
-
\$\begingroup\$ This looks like example code... are you just toying around or are you planning an implementation of this? \$\endgroup\$Pimgd– Pimgd2015年01月30日 10:40:43 +00:00Commented 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\$sjonniesjon– sjonniesjon2015年01月30日 10:47:30 +00:00Commented 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– Pimgd2015年01月30日 10:48:37 +00:00Commented 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– Pimgd2015年01月30日 10:54:56 +00:00Commented 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– rolfl2015年01月30日 16:47:25 +00:00Commented Jan 30, 2015 at 16:47
1 Answer 1
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.
-
\$\begingroup\$ Ok, makes a lot of sense. I'll modify the inheriting class and move the constructor logic to a separate initialization function. \$\endgroup\$sjonniesjon– sjonniesjon2015年01月30日 14:24:46 +00:00Commented Jan 30, 2015 at 14:24