Form validation library

Question

Background:

After reading this thought provoking book I decided to write a small library as an exercise.

goog.require('goog.structs.Map');
var MyModule = {};
MyModule.FormHelper = {
 /** @type {?Element} */
 form: null,
 /** @type {goog.structs.Map<string, string>} */
 errors: {},
 /** @param {!Element} element */
 init: function(formElement) {
 this.form = formElement;
 },
 /**
 * @param {!Element} element
 * @return {!Array<goog.structs.Map<string, Object>>}
 */
 getElementAttributes: function(element) {
 if (!element) return [];
 return [].slice.call(element.attributes).map(function(attr) {
 return {[attr.name]: attr.value};
 });
 },
 /**
 * @param {string} filterOn
 * @return {!Array<!Element>}
 */
 filterElements: function(filterOn) {
 return [].slice.call(this.form.elements).filter(function(element) {
 return element.hasAttribute(filterOn);
 });
 }
};
MyModule.Form = Object.create(MyModule.FormHelper);
MyModule.Form.validate = function() {
 /** @type {!Array<!Element>} */
 var elements = this.getRequiredElements();
 elements.reduce(function(map, element) {
 if (element.value == '') {
 map[element.name] = 'Required field cannot be empty';
 }
 return map;
 }, this.errors);
};
MyModule.Form.getRequiredElements = function() {
 return this.filterElements('required');
};
MyModule.createForm = function(name) {
 /** @type {!Element} */
 var formElement = document.forms[name];
 if (!formElement) {
 throw new Error('Form not found.');
 }
 var form = Object.create(MyModule.Form);
 form.init(formElement);
 return form;
};
var contactForm;
var callback = function(e) {
 e.preventDefault()
 contactForm.validate();
 console.log(contactForm.errors);
}
window.onload = function () {
 contactForm = MyModule.createForm('ContactForm');
 contactForm.form.addEventListener("submit", callback, false); //Modern browsers
}

I am using Google closure for type checking and advance code optimizations.

I want to check if I am following a good object oriented design and practices?

Update 1

;(function(global, doc, lib) {
 var Form = {};
 Form.create = function(name) {
 var form = doc.forms[name];
 if (!form) {
 throw new Error('Form not found.');
 }
 this.formElement = form;
 };
 Form.elements = function() {
 return this.formElement.elements;
 };
 Form.filter = function(attr) {
 return [].slice.call(this.elements()).filter(function(element) {
 return element.hasAttribute(attr);
 });
 };
 Form.getAttributes = function(element) {
 if (!element) return [];
 return [].slice.call(element.attributes).map(function(attr) {
 return {[attr.name]: attr.value};
 });
 };
 Form.validate = function(formName) {
 var elements = this.filter('required');
 elements.reduce(function(error, element) {
 if (element.value == '') {
 error[element.name] = 'Required field cannot be empty';
 }
 return error;
 }, this.errors);
 };
 lib.validate = function(formName) {
 var form = Object.create(Form);
 form.create(formName);
 form.errors = [];
 form.validate();
 return form;
 };
})(window, document, window.lib || (window.lib = {}));
window.onload = function() {
 console.log(lib);
 console.log(lib.validate('foo').errors);
};

• gcampbell: Are you using ES6 or not?
• CodeYogi: @gcampbell no! not in near future.
• gcampbell: {[attr.name]: attr.value } is an ES6 object literal.

Answer 1

Global variables

I know this isn't part of the module and probably just throwaway code, but you should restructure the module usage to avoid making global variables contactForm and callback:

 window.onload = function () {
 var contactForm = MyModule.createForm('ContactForm');
 contactForm.form.addEventListener("submit", function onSubmit(e) {
 e.preventDefault()
 contactForm.validate();
 console.log(contactForm.errors);
 }, false); //Modern browsers
 }

Organization of properties between prototypes

I'm not sure why you'd divide up methods and properties between FormHelper and Form, as they seem to perform very similar functionality. I'd combine them into a single Form object; that's the only prototype you need.

This may seem too simple, but your module solves a simple problem. A more complicated module that would benefit from delegation between multiple prototypes would be one that has multiple types of Element prototypes, with some validation functions shared between Elements and some specific to particular types.

form prototype property

I don't think you need/want the form: null property on the prototype object. Even though init is a method of the prototype, it's assigning form to the base object (also named form) that exists in the submit handler, and that's what you want - a shared form property would make multiple Forms impossible.

Also, a subjective point: I'd rename this property to formElement or just element, since there are too many forms and Forms running around this code.

---

I hope this helps! Kyle Simpson's YDKJS books made me think as well, and with ES6 classes growing more common every day, I often wonder how this explicitly prototype-based JavaScript style can coexist with the classes Simpson discredited.

• CodeYogi: Hi can you please provide some more insight? is this code production ready?
• Aaron: What kind of additional info are you interested in? I would say the code isn't production ready until you fix the first issue I described, as modules shouldn't pollute the global namespace. The internal structure isn't a deal-breaker if the module works, but splitting it into Form and FormHelper makes it somewhat trickier to maintain.
• CodeYogi: In that case I need to update my question then.
• Aaron: Let me know when you do, and I'll take a look and post a new answer.
• CodeYogi: Updated, but I think I am getting more confused about I need an object or not.

Answer 2

Your updated code gets to the heart of the matter, and we start to see how simple this module is - really, it's just validating required form elements. You could also remove getAttributes, since it's currently unused.

As for your question, no, you don't really need an object, especially if the library is going to stay this simple. You could simplify the code by:

• taking out Form entirely
• deleting Form.create, and
• converting all other Form functions into private functions that are called by each other and by lib.validate directly.

Your current object-oriented approach would come in handy in a few situations, but for each you'd have to export Form itself as lib.Form, not just lib.validate.

One situation is if you ever wanted to extend this base Form into different types of Forms; then the prototype delegation would allow these different types to share common Form functions.

Another situation is if you changed the API to be more jQuery-like: users would be able to wrap a form in a Form object once, then call validate on it multiple times. This is a very different API (and makes the most sense for libraries like jQuery that do a lot of different things). The disadvantage of this approach is that it's more verbose (var form = lib.Form('foo'); console.log(form.validate().errors) versus just console.log(lib.validate().errors)), while the advantage is that createForm doesn't have to be called every time validate is.

One small note: simple functions like Form.elements can be defined via "getter" functions for a slightly cleaner look:

Object.defineProperty(Form, "elements", { get: function () { this.formElement.elements; } });
Form.filter = function(attr) {
 // NOTE: this.elements instead of this.elements()
 return [].slice.call(this.elements).filter(function(element) {
 return element.hasAttribute(attr);
 });
};

• CodeYogi: Regarding elements access I would go for UFP wdyt?
• Aaron: Sure, that makes sense. I've usually gone with methods (in this case, named getElements), but sometimes these feel like excessive boilerplate. You also have a formElement property, but it could be considered private, i.e. not one of Form's "services".