A few things immediately caught my eye:
You're using a weird construct:
new function() {}
. Thefunction(){}
expression already creates a new function, but in addition to that you're using it withnew
to invoke it as a constructor for new object. The way you're using it, you could easily replace it with just plain object literal. For example injs_util
:js_util = { isWhitespaceCharacter: function(ch) { return (ch == ' ' || ch == '\n' || ch == '\r' || ch == '\t'); }, ... };
But you really need to familiarize yourself with JavaScript's prototypal object model. Otherwise you're just programming by coincidence - without really knowing what the code does.
Don't use the
with
statement. It makemakes code hard to read and reason about. It's one of the grand mistakes in JavaScript language. Avoid it. In your case, just prefix the instance variables/functions withthis.
- it's not so long to write.It's very unusual to place the opening
{
on a separate line in JavaScript. This kind of indentation style is rarely seen. I suggest you adopt the common style of placing{
on the same line asif
etc.The common variable naming convention in JavaScript is to use
camelCase
notunder_scores
.Your
is_whitespace
function could be much more easily implemented with just a simple regular expression:isWhitespace: function(s) { return /^\s*$/.test(s); }
And don't take my word for it. Run your JavaScript through an analyzer like JSHint - it will warn you about these and other issues.
PS. I think overall your code is very nicely and cleanly written. It just falls short on looking and feeling like JavaScript.