3
\$\begingroup\$

I wrote a simple JSON schema validator. The full code is over here on gist.github.com However, the code without comment is...

validate = function(schema, instance) {
    var i;
    var errors = 0;

    var getType = function(attr) {
        return Object.prototype.toString.call(attr);
    }

    var addError = function(msg, attrs) {
        console.error(msg, attrs);
        errors += 1;
    }

    if(getType(schema) !== getType(instance)) {
        addError("Type Mismatch", [schema, instance]);
        return errors;
    }

    for(i in schema) {
        if(schema.hasOwnProperty(i)) {

            if(instance[i] == undefined) {
                addError("Property Not found", i);

            }
            //Special Handling for arrays
            else if( getType(schema[i]) === getType([]) ) {
                var zeroSchema = schema[i][0];
                var zeroInstance = instance[i][0];
                if(zeroInstance === undefined) {
                    continue;
                }
                for(var j=0;j<instance[i].length;j++) {
                    errors += validate(zeroSchema, instance[i][j]); 
                }

            }
            //Special Handling for nested objects
            else if( getType(schema[i]) === getType({}) ) {
                errors += validate(schema[i], instance[i]);
            }

        }
    }
    return errors;

}

The code doesn't need the schema object to explicitly specify the type of object properties unlike the official one

I'm fairly new to javascript. How do I improve the code?

\$\endgroup\$
  • \$\begingroup\$ Edited your question putting the code from the link into it. \$\endgroup\$ – BDM Mar 15 '13 at 11:30
  • \$\begingroup\$ You could pass your script through Javascript Lint to check for missing semicolons and things like that. \$\endgroup\$ – Sjoerd Mar 15 '13 at 12:23
  • 2
    \$\begingroup\$ @palacsint Thanks. ProfPickle helped add the code \$\endgroup\$ – Pramod Mar 15 '13 at 12:49
3
\$\begingroup\$

I'd definitely change:

if(instance[i] == undefined) {
    addError("Property Not found", i);
}

to something more robust, e.g.

if (!instance.hasOwnProperty(i)) {
    addError("Property Not found", i);
}

Stylistically I'd change a few things as well:

Add a space between function names and (): function() to function ()
Add a space between if, for etc and (: for(i in schema) to for (i in schema)
Add in semi-colons after defining your functions:

var getType = function(attr) {
    return Object.prototype.toString.call(attr);
}; // <-- here.

Also, because JavaScript doesn't have block scope I like to define all my variables at the top of the function. e.g. you have:

validate = function (schema, instance) {

    // omitted code.

    for(var j=0; j<instance[i].length; j++) {
        errors += validate(zeroSchema, instance[i][j]); 
    }

    // more code.
}

Whereas I would prefer j defined at the top of the function (as well as the extra spaces I've added).

Having said all of that, I wonder whether your solution is flexible enough - how would you specify an optional property? A property with a minimum and/or maximum value?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.