First, I would like to clarify a common misconception. Forgive me if you clearly understood this; perhaps it will be useful for someone else.
Learning and using jQuery or a similar library does not exclude or conflict with learning JavaScript. jQuery is just a DOM manipulation library that removes many pain points when using the DOM. There is plenty of room for learning and using JavaScript, the language, even if you use the library to abstract out some of the details of the DOM.
In fact, I would say that using the DOM directly is likely to teach bad JavaScript coding, because the DOM is not a JavaScript ish API. It was designed to work identically in JavaScript and Java, and possibly in other languages, and therefore it does not fully utilize the capabilities of the JavaScript language.
Of course, as you said, you use this as an exercise for learning; I just don’t want you to fall into the trap that I saw when many people think: “I don’t want to learn jQuery because I want to learn JavaScript!” This is a false dichotomy: you should learn JavaScript anyway, and using jQuery for the DOM doesn't interfere at all.
Now some details ...
While OK, to indicate property names in an object literal, and when you reference properties, it is normal - and more readable - not to specify them when they are valid JavaScript names. for example in your formElement object
formElement = { id: {}, name: {}, value: {}, required: {}, type: {} };
(there was also no semicolon at the end)
and where do you use names you can do:
formElement.id[i] = ... formElement.name[i] = ...
and etc.
Do not loop back if this does not require program logic. This does not make the code faster, except possibly in the case of an extremely tight loop, and it makes it unclear whether you are just prematurely optimizing or really need a back loop.
Speaking of optimization, this loop has several inArray() calls. Since each of these loops is through an array, this may be a performance impact rather than an external loop. I suppose these arrays are probably pretty short? Thus, performance does not matter at all, but you can think about it in cases where you have more arrays and objects. In some cases, you can use an object with property names and values for faster searches - but I did not look very carefully at what you are doing to offer something.
In any case, you are using inArray() incorrectly! But not your fault, this is a ridiculously named function in jQuery. The name explicitly offers a boolean return value, but the function returns an array index with a null value, or -1 if no value is found. I highly recommend renaming this function as indexOf() to match the built-in Array or arrayIndex() method, or some.
In the same loop, form[i] repeated many times. You can do this at the top of the loop:
var field = form[i];
and then use field everywhere, for example. field.id instead of form[i].id . This is usually faster if it matters (which is probably not here), but, more importantly, it is easier to read.
Do not use strict logical comparisons such as if( foo === true ) and if( bar === false) unless you really need to - and these cases are rare. The code sends a signal to the reader that something is happening that is different from the usual Boolean test. The only time these specific tests should be used is when you have a variable that may contain a boolean or may contain some other type of value, and you need to distinguish what exactly.
A good example of when you should use tests like this is an optional parameter, which defaults to true :
// Do stuff unless 'really' is explicitly set to false, eg // stuff(1) will do stuff with 1, but stuff(1,false) won't. function stuff( value, really ) { if( really === false ) { // don't do stuff } else { // do stuff } }
This specific example does not make much sense, but it should give you this idea.
Similarly, the test === true can be used when it is necessary to distinguish between the actual value of boolean true and some other "true" value. Indeed, it looks like this line is a valid example for this:
if (formElement['required'][i] === true){
given that if (formElement['required'][i] comes from the getDataAttribute() function, which can return a Boolean or other type.
If you are just checking the veracity - and this should be in most cases - just use if( foo ) or if( ! foo ) . Or similarly in the conditional expression: foo ? x : y foo ? x : y or !foo ? x : y !foo ? x : y .
Above was a long way to say that you should change this:
if (empty_content === false) {
in
if (!empty_content) {
Your getFormParam() function goes into some work to convert the result undefined to null . There is usually no reason for this. I don’t see the place where this function is called, so I can’t concretize it specifically, but in general you will test the veracity of something like this, so null and undefined will be processed as false . Or in cases where you need to distinguish between null / undefined from other values (for example, explicit false ), you can easily do this with != null or == null . This is one of the cases where the “weaker” comparison made using == and != very useful: both null and undefined evaluate the same with these operators.
You asked to ignore the coding style, but one small suggestion here: you have a combination of camelCaseNames and names_with_underscores . In JavaScript, camelCaseNames more idiomatic for function names and variables, PascalCaseNames for constructor functions. Of course, feel free to use underscores where they make more sense, for example, if you write code that works with database columns in this format, you might want your variable names to match the column names.
Hope this helps! Keep up the good work.
Update for new code
I have problems with the code logic, and I think I know part of the reason. It is a combination of naming conventions and objects inside out.
Firstly, the name formElement really confusing. When I see element in JavaScript, I think of a DOM element (HTMLElement) or an array element. I'm not sure if this formElement one or the other or not.
So, I look at the code to find out what it does, and I see that it has the id:{}, name:{}, ... properties, but later the code treats each of them as Array , not Object :
formElement.id[i] = ... formElement.name[i] = ... formElement.value[i] = ... formElement.required[i] = ... formElement.type[i] = ...
(where i is an integer index)
If this code is right, they should be arrays instead of: id:[], name:[], ...
But this is a red flag. When you see that you are creating parallel arrays in JavaScript, you are probably mistaken. In most cases, you are better off replacing parallel arrays with one array of objects. Each of the objects in this array is a single slice through all of your parallel arrays with a property for each of the previous arrays.
So this object (where I made the correction from {} to [] to match its current usage):
formElement = { id: [], name: [], value: [], required: [], type: [] };
it should be:
formInfo = [];
and then where you have the code:
formElement.id[i] = ...; formElement.name[i] = ...; formElement.value[i] = ...; formElement.required[i] = ...; formElement.type[i] = ...;
It should be:
var info = { id: ..., name: ..., value: ..., required: ..., type: ... }; formInfo.push( info );
and adjust the rest of the code. For instance:
formElement.required[i]
:
formInfo[i].required
or even simpler as it is in the same function:
info.required
And note: I'm not saying that info and formInfo are great names :-) they are just placeholders, so you can think of a better name. The main idea is to create an array of objects instead of a set of parallel arrays.
Last, and then I have no time.
This getDataAttribute() function is a tricky little job. You do not need it! It would be easier to just call the base function directly where you need it:
var info = { ... required: formField.getAttribute('data-required') === 'true', type: formField.getAttribute('data-type') };
It also gives you full control over the interpretation of attributes - as in the test === 'true' above. (This gives you the correct boolean value, so when testing the value later you do not need to use === true on it.)
On a stylistic note, yes, I made a hard code of the two names 'data-xxxx' right there, and I think this is a better and more understandable way to do this. Don't let your C experience drop you here. There is no advantage in defining the string "constant" in this particular case, unless that is what you want to make custom, and it is not.
Also, even if you create a string constant, the slight advantage is that you have the complete string 'data-whatever' instead of 'whatever' . The reason is that when someone reads your HTML code, they can see a string in it and look for JS code for that string. But when they search for data-whatever , they will not find it if the data- prefix data- automatically added to the JS code.
Oh, I forgot the last. This code:
function Form(){ var args = Array.prototype.slice.call(arguments), formName = args[0], callback = args.pop(),
works too hard! Just do it instead:
function Form( formName, callback ) {
(and save var for the rest of the variable declarations, of course)