Javascript Form Manipulator Code Review

I'd appreciate a review of at least some of this javascript, if anyone has time. It's really long, but I think it's useful. It's designed to let you create forms and do javascripty things by only modifying html tags and attributes.

Take a look at what it's supposed to do here: https://docs.google.com/document/d/1T08AJr4OGR2ohXLeSK3WjKn8zG-sSLi3Vz1Ya5tLkQY/edit?usp=sharing

```
//Configuration variables
var config = jQuery("edfconfig");

function getConfigBoolean(attr) {
if (config != null)
return jQuery(config).attr(attr) != undefined;
}
function getConfigValue(attr) {
if (config != null)
return jQuery(config).attr(attr);
}

var noasterisks = getConfigBoolean("noasterisks");
var addafter = getConfigBoolean("addafter");
var doactions = getConfigBoolean("doactions");
var noappend = getConfigBoolean("noappend");
var requiredmessage = getConfigValue("requiredmessage");
var requiredmessageid = getConfigValue("requiredmessageid");

//eDomForm variables
var attrs = jQuery("edfvars")[0];
var variables = {};
if (attrs != null)
for (i=0;iBack Next');
});

//Next and Back buttons
var pages = jQuery(".form_page");
var backButtons = jQuery(".back");
var nextButtons = jQuery(".next");
for (i=0;i -1;
}

var aliases = jQuery(".alias");

//Connect all aliases and original fields to each other
jQuery(".alias").each(function(i){
var alias = jQuery(this).attr("alias");
var allaliases = jQuery("[alias="+alias+"]");
var alloriginals = jQuery("."+alias);
jQuery(alloriginals).each(function(j){
var thisOriginal = this;
jQuery(allaliases).each(function(k) {
var thisAlias = this;
if (hasClass(thisOriginal,"required_field")) {
jQuery(thisAlias).addClass("required_field");
}
thisAlias.onchange = aliasChange.bind(null,thisAlias,thisOriginal,thisAlias.onchange);
thisOriginal.onchange = aliasChange.bind(null,thisOriginal,thisAl

After looking through your code I have a few suggestions for you that are mainly best practice type suggestions:

• Avoid global variables

You are defining just about all of your code at the global scope which is a bad practice that can lead to conflicts with other scripts. For instance, you have defined config as a global variable. That is a really common variable name and another script may use a global variable named config which could cause problems with both scripts.

Consider at the very least defining all of your code inside a self-executing function:

(function(){/Your code/});

Or even better create a namespace and define all of your variables and functions there:

var yourNamespace = {
 YourFunctionName : function(){},
 config : {}
}

• Always use brackets after if statements

In several place you are omitting the {} after if statements. This is perfectly valid JavaScript, but it creates some readability issues and can also lead to bugs. Lets say in the future you require two statements after your if statement and you forget to add the braces. Anyway, the point is there is no good reason not to put in the braces.

• Use !== and === instead of != and '=='

There is no good reason to use != and ==. When testing for equality or inequality you should be checking for type and value equality. Using the != and == can lead to surprising results that are not at all intuitive.

• Use a single var keyword

This is a bit of a style suggestion that most JS developers adhere to but is by no means absolutely necessary. Again, this will make your code look better to many developers but is functionally equivalent to what you have.

var noasterisks         = getConfigBoolean("noasterisks"),
    addafter            = getConfigBoolean("addafter"),
    doactions           = getConfigBoolean("doactions"),
    noappend            = getConfigBoolean("noappend"),
    requiredmessage     = getConfigValue("requiredmessage"),
    requiredmessageid   = getConfigValue("requiredmessageid");

This is not an exhaustive list, but the first three suggestions will make your code more maintainable and easier to debug than it stands.

StackExchange Code Review Q#27915, answer score: 3