Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Its only a small Quibble with your code but I find the naming of formObj a little odd. For me I see this as a FormValidator or FormExtender or some such.

Another point would be the for(i in ....) should be for(var i in ...) it will run either way and I believe it is just a matter of coding style.

Also you have functions like:

validate: function(theForm, required({....

I think it helps debugging if you do this instead:

validate: function FormValidate(....

It has the disadvantage of polluting the namespace but makes debugging easier as the function is now not anonymous.

If you were making this a class to instantiate you would write it like so:

var FormValidator = function(theForm)
{
 this.formElement = theForm;
 return this;
}
FormValidator.prototype = {
 validate : function(theForm, required) {
 //...
 },
 inputListener : function(theField) {
 //...
 },
 ajaxSubmit : function(formToSubmit) {
 //...
 }
};
var mainForm = new FormValidator(document.forms[0]);

If you were doing a Singleton pattern something like:

var FormValidator = function()
{
 return this;
}
function GetFormValidatorInstance()
{
 return window.__formValidatorInstance || new FormValidator();
}

These last two are probably overkill if you just want a simple Component. A more in depth example of the above can be found at http://codereview.stackexchange.com/questions/1658/javascript-code-class-structure https://codereview.stackexchange.com/questions/1658/javascript-code-class-structure

Its only a small Quibble with your code but I find the naming of formObj a little odd. For me I see this as a FormValidator or FormExtender or some such.

Another point would be the for(i in ....) should be for(var i in ...) it will run either way and I believe it is just a matter of coding style.

Also you have functions like:

validate: function(theForm, required({....

I think it helps debugging if you do this instead:

validate: function FormValidate(....

It has the disadvantage of polluting the namespace but makes debugging easier as the function is now not anonymous.

If you were making this a class to instantiate you would write it like so:

var FormValidator = function(theForm)
{
 this.formElement = theForm;
 return this;
}
FormValidator.prototype = {
 validate : function(theForm, required) {
 //...
 },
 inputListener : function(theField) {
 //...
 },
 ajaxSubmit : function(formToSubmit) {
 //...
 }
};
var mainForm = new FormValidator(document.forms[0]);

If you were doing a Singleton pattern something like:

var FormValidator = function()
{
 return this;
}
function GetFormValidatorInstance()
{
 return window.__formValidatorInstance || new FormValidator();
}

These last two are probably overkill if you just want a simple Component. A more in depth example of the above can be found at http://codereview.stackexchange.com/questions/1658/javascript-code-class-structure

Its only a small Quibble with your code but I find the naming of formObj a little odd. For me I see this as a FormValidator or FormExtender or some such.

Another point would be the for(i in ....) should be for(var i in ...) it will run either way and I believe it is just a matter of coding style.

Also you have functions like:

validate: function(theForm, required({....

I think it helps debugging if you do this instead:

validate: function FormValidate(....

It has the disadvantage of polluting the namespace but makes debugging easier as the function is now not anonymous.

If you were making this a class to instantiate you would write it like so:

var FormValidator = function(theForm)
{
 this.formElement = theForm;
 return this;
}
FormValidator.prototype = {
 validate : function(theForm, required) {
 //...
 },
 inputListener : function(theField) {
 //...
 },
 ajaxSubmit : function(formToSubmit) {
 //...
 }
};
var mainForm = new FormValidator(document.forms[0]);

If you were doing a Singleton pattern something like:

var FormValidator = function()
{
 return this;
}
function GetFormValidatorInstance()
{
 return window.__formValidatorInstance || new FormValidator();
}

These last two are probably overkill if you just want a simple Component. A more in depth example of the above can be found at https://codereview.stackexchange.com/questions/1658/javascript-code-class-structure

deleted 10 characters in body
Source Link
James Khoury
  • 3.2k
  • 1
  • 25
  • 51

Its only a small Quibble with your code but I find the naming of formObj a little odd. For me I see this as a FormValidator or FormExtender or some such.

Another point would be the for(i in ....) should be for(var i in ...) it will run either way and I believe it is just a matter of coding style.

Also you have functions like:

validate: function(theForm, required({....

I think it helps debugging if you do this instead:

validate: function FormValidator$validateFormValidate(....

It has the disadvantage of polluting the namespace but makes debugging easier as the function is now not anonymous.

If you were making this a class to instantiate you would write it like so:

var FormValidator = function(theForm)
{
 this.formElement = theForm;
 return this;
}
FormValidator.prototype = {
 validate : function(theForm, required) {
 //...
 },
 inputListener : function(theField) {
 //...
 },
 ajaxSubmit : function(formToSubmit) {
 //...
 }
};
var mainForm = new FormValidator(document.forms[0]);

If you were doing a Singleton pattern something like:

var FormValidator = function()
{
 return this;
}
function GetFormValidatorInstance()
{
 return window.__formValidatorInstance || new FormValidator();
}

These last two are probably overkill if you just want a simple Component. A more in depth example of the above can be found at http://codereview.stackexchange.com/questions/1658/javascript-code-class-structure

Its only a small Quibble with your code but I find the naming of formObj a little odd. For me I see this as a FormValidator or FormExtender or some such.

Another point would be the for(i in ....) should be for(var i in ...) it will run either way and I believe it is just a matter of coding style.

Also you have functions like:

validate: function(theForm, required({....

I think it helps debugging if you do this instead:

validate: function FormValidator$validate(....

It has the disadvantage of polluting the namespace but makes debugging easier as the function is now not anonymous.

If you were making this a class to instantiate you would write it like so:

var FormValidator = function(theForm)
{
 this.formElement = theForm;
 return this;
}
FormValidator.prototype = {
 validate : function(theForm, required) {
 //...
 },
 inputListener : function(theField) {
 //...
 },
 ajaxSubmit : function(formToSubmit) {
 //...
 }
};
var mainForm = new FormValidator(document.forms[0]);

If you were doing a Singleton pattern something like:

var FormValidator = function()
{
 return this;
}
function GetFormValidatorInstance()
{
 return window.__formValidatorInstance || new FormValidator();
}

These last two are probably overkill if you just want a simple Component. A more in depth example of the above can be found at http://codereview.stackexchange.com/questions/1658/javascript-code-class-structure

Its only a small Quibble with your code but I find the naming of formObj a little odd. For me I see this as a FormValidator or FormExtender or some such.

Another point would be the for(i in ....) should be for(var i in ...) it will run either way and I believe it is just a matter of coding style.

Also you have functions like:

validate: function(theForm, required({....

I think it helps debugging if you do this instead:

validate: function FormValidate(....

It has the disadvantage of polluting the namespace but makes debugging easier as the function is now not anonymous.

If you were making this a class to instantiate you would write it like so:

var FormValidator = function(theForm)
{
 this.formElement = theForm;
 return this;
}
FormValidator.prototype = {
 validate : function(theForm, required) {
 //...
 },
 inputListener : function(theField) {
 //...
 },
 ajaxSubmit : function(formToSubmit) {
 //...
 }
};
var mainForm = new FormValidator(document.forms[0]);

If you were doing a Singleton pattern something like:

var FormValidator = function()
{
 return this;
}
function GetFormValidatorInstance()
{
 return window.__formValidatorInstance || new FormValidator();
}

These last two are probably overkill if you just want a simple Component. A more in depth example of the above can be found at http://codereview.stackexchange.com/questions/1658/javascript-code-class-structure

Source Link
James Khoury
  • 3.2k
  • 1
  • 25
  • 51

Its only a small Quibble with your code but I find the naming of formObj a little odd. For me I see this as a FormValidator or FormExtender or some such.

Another point would be the for(i in ....) should be for(var i in ...) it will run either way and I believe it is just a matter of coding style.

Also you have functions like:

validate: function(theForm, required({....

I think it helps debugging if you do this instead:

validate: function FormValidator$validate(....

It has the disadvantage of polluting the namespace but makes debugging easier as the function is now not anonymous.

If you were making this a class to instantiate you would write it like so:

var FormValidator = function(theForm)
{
 this.formElement = theForm;
 return this;
}
FormValidator.prototype = {
 validate : function(theForm, required) {
 //...
 },
 inputListener : function(theField) {
 //...
 },
 ajaxSubmit : function(formToSubmit) {
 //...
 }
};
var mainForm = new FormValidator(document.forms[0]);

If you were doing a Singleton pattern something like:

var FormValidator = function()
{
 return this;
}
function GetFormValidatorInstance()
{
 return window.__formValidatorInstance || new FormValidator();
}

These last two are probably overkill if you just want a simple Component. A more in depth example of the above can be found at http://codereview.stackexchange.com/questions/1658/javascript-code-class-structure

default

AltStyle によって変換されたページ (->オリジナル) /