Are there any flaws in my class implementation?

I started writing a class implementation for my JavaScript framework, which I’m using on my sites as a lightweight toolkit when I don’t need something heavyweight like MooTools or jQuery. I decided to write a class implementation mainly to make inheritance easier. This was heavily inspired by the [url=http://mootools.net/docs/core/Class/Class]Class system in MooTools, [url=http://ejohn.org/blog/simple-javascript-inheritance/]John Resig’s Simple JavaScript Inheritence, [url=http://dean.edwards.name/weblog/2006/03/base/]Dean Edwards’ Base.js and [url=http://myjs.fr/my-class/]My.js. I’d like someone experienced to go over the code and let me know if it has any issues.

Here’s the code so far:


/**
 * Daniel15 JavaScript Framework - By Daniel15, 2012
 * http://dl.vc/jsframework
 * Feel free to use any of this, but please link back to my site (dan.cx)
 */

(function()
{
	/**
	 * Create a new class instance
	 * @param	Object		Functions to add to the class
	 * @param	Class		Class to extend, if any
	 * @return	Class instance
	 */
	var Class = window.Class = function(proto, extendThis)
	{
		// Create the class constructor
		//var NewClass = proto.init ? proto.init : function() {};
		function NewClass()
		{				
			if (this.init)
				this.init.apply(this, arguments);
		}
		NewClass.extend = Class.extend;
		
		// If not extending, easy - Just assign the prototype directly
		if (!extendThis)
		{
			NewClass.prototype = proto;
			return NewClass;
		}
		
		// Extending - Create an instance of the parent class to use as the prototype
		// But *don't* run the constructor (creates an empty constructor first)
		function ParentClassEmpty() {}
		ParentClassEmpty.prototype = extendThis.prototype;
		NewClass.prototype = new ParentClassEmpty;
		
		// Add all the new methods to the prototype
		for (var name in proto)
		{
			if (!proto.hasOwnProperty(name))
				return;
				
			// Does this function already exist on the parent class?
			if (extendThis.prototype[name])
				NewClass.prototype[name] = Class.wrapFn(extendThis.prototype[name], proto[name]);
			else
				// No parent method with the same name, so we can just copy it directly
				NewClass.prototype[name] = proto[name];
		}
		
		return NewClass;
	};
	
	// Add useful functions to the Class object
	Util.extend(Class,
	{
		/**
		 * Extend the current class. Expected to be called in the context of an existing class.
		 * @param	Object		Prototype methods of new class
		 * @return	Class instance
		 */
		extend: function(proto)
		{
			return new Class(proto, this);
		},
		/**
		 * Create a wrapper for this function. This wrapper saves the parent function into .parent
		 * (so it can be called from the overriding function) before calling the overriden function.
		 * @param	Function	Function on the superclass
		 * @param	Function	Function on the overriding class
		 * @return	Function
		 */
		wrapFn: function(superFn, fn)
		{
			return function()
			{
				// Save the current parent method
				var tmp = this.parent;
				this.parent = superFn;
				
				var ret = fn.apply(this, arguments);
				
				// Restore whatever was in .parent before
				this.parent = tmp;
				return ret;
			};
		}
	});
})();

And here’s an example usage:


var Person = new Class(
{
	init: function(name)
	{
		//console.log('Init person');
		this.name = name;
	},
	helloWorld: function()
	{
		return 'Hello';
	},
	helloWorld2: function()
	{
		return 'Hello2';
	}
});


var me = new Person('Daniel');
console.log('me.name = ', me.name); // Daniel
console.log('me.helloWorld() = ', me.helloWorld()); // Hello
console.log('me.helloWorld2() = ', me.helloWorld2()); // Hello2
console.log('me instanceof Person = ', me instanceof Person); // true

var Ninja = Person.extend(
{
	helloWorld2: function()
	{
		return 'Ninja helloWorld2, original = ' + this.parent();
	},
	helloWorld3: function()
	{
		return 'Ninja helloWorld3';
	}
});
var awesome2 = new Ninja('Awesome');
console.log('awesome2.name = ', awesome2.name); // Awesome
console.log('awesome2.helloWorld() = ', awesome2.helloWorld()); // Hello
console.log('awesome2.helloWorld2() = ', awesome2.helloWorld2()); // Ninja helloWorld2, original = Hello2
console.log('awesome2.helloWorld3() = ', awesome2.helloWorld3()); // Ninja helloWorld3
console.log('awesome2 instanceof Ninja = ', awesome2 instanceof Ninja); // true
console.log('awesome2 instanceof Person = ', awesome2 instanceof Person); // true

Util.extend is simply:


	/**
	 * Add all elements from source to destination and return the modified destination (for chaining)
	 * @param	Object to copy properties to
	 * @param	Object to copy properties from
	 */
	extend: function(destination, source)
	{
		for (var key in source || {})
		{
			if (source.hasOwnProperty(key))
				destination[key] = source[key];
		}
		return destination;
	},

Thoughts?

Far be it for me to step heavily on such work, but JavaScript has Object.create() for creating objects already.

But first, starting curly braces on a new line in most other programming languages doesn’t cause problems. With JavaScript though that does definitely cause problems.
Take this code as an example.


return
{
    value: true
}

Putting the curly brace on the left in JavaScript does definitely cause interpretation problems. Here’s how:

[list][]JavaScript has something called automatic semicolon insertion, which when it sees the return keyword with nothing after it, inserts a semicolon there, which results in it returning undefined
[
]The curly brace can mean to start an object literal, or it can mean a block.
[]The value keyword then becomes a statement label
[
]That which was intended to be assigned to the keyword now becomes just a useless expression statement
[*]So automatic semicolon insertion automatically adds one after the useless expression statement[/list]


return[color="red"];[/color]          [color="grey"]// semicolon insertion[/color]
{ [color="grey"]               // block[/color]
    value: true[color="red"];[/color] [color="grey"]// statement label, useless expression statement, and semicolon insertion[/color]
}

When a styalistic choice results in such problems, it’s best to use a style that guarantees that you will never face such problems, ever.
Start the curly brace on the right instead.


return {
    value: true
}

Full details about this are explained in the following video - JavaScript: The Good Parts (30m39s in to it)

So far as your code goes, classical inheritance is really just patched on to JavaScript so that people coming over from other languages could have something that they are familiar with from their other languages.

When you’re programming in a language, it really does help to program in that language. Not in some other language.

The real power of JavaScript comes from prototypal inheritance. Take a look at Object.create, There’s some very good material on this in the [url=“http://www.yuiblog.com/blog/2010/02/24/video-crockonjs-3/”]Function the ultimate presentation ([url=“http://www.slideshare.net/douglascrockford/3-7687071”]slides)

Here’s the latest version of my code: https://github.com/Daniel15/JSFramework/blob/classes/class.js

But first, starting curly braces on a new line in most other programming languages doesn’t cause problems. With JavaScript though that does definitely cause problems.
Take this code as an example.

Thanks Paul. I use this particular style in JavaScript since it’s what I use in the other languages I use most of the time (PHP and C#) and I normally don’t have issues (except for return statements, as you outlined). I do use the “brace on the same line” syntax in some situations (eg. see the “new Class” calls in my code). I am aware of semicolon insertion and the issues surrounding it. Each to their own… Style opinions like this are almost as strong as arguments about tabs vs spaces. :slight_smile:

The real power of JavaScript comes from prototypal inheritance.

Prototypal inheritance is what I’m trying to make easier, just adding “syntax sugar”. Given this:


function Person(name)
{
        // constructor
}

Person.prototype =
{
        helloWorld: function()
        {
                // ...
        }
};


function Ninja(name)
{
        Person.apply(this, arguments);
}

Ninja.prototype = Object.create(Person.prototype);

Ninja.prototype.helloWorld = function()
{
        Person.prototype.helloWorld.apply(this, arguments);
        // more stuff
};
// Add functions - Ninja.prototype.blah = function() { ... }

And this:


var Person = new Class({
        init: function()
        {
                // constructor
        },
        helloWorld: function()
        {
                // ...
        }
});

var Ninja = Person.extend({
        init: function()
        {
                // more init
        },
        helloWorld: function()
        {
                this.super();
                // more stuff
        }
        // Add functions - blah: function() {...}
});

I prefer the second one, I think it looks nicer. Convention over configuration wins here. Underneath they’re both the same, the Class object is just “syntax sugar”. Again, each to their own, but I like nice syntax with less boilerplate code.

There’s some very good material on this in the Function the ultimate presentation (slides)

Thanks for that! Slide 53 is essentially the same thing I’m doing, except with more concise code.

In other programming languages it is a style opinion that doesn’t affect the code.

JavaScript is unique in that where you place the opening brace determines where you will have that problem or not.
When it comes to issues such as that, it makes more sense to code in a style that guarantees your code will never face the problem. Ever.

In those other programming languages it is a style. In JavaScript, it’s not a convention of style. It actually changes how your code is interpreted by he web browser.

I know that you’re used to one style in another programming language. You’re programming in JavaScript now.
It’s time for you to write your code in the language that you’re writing it in.

Apart from object literals in return statements, I have never had an issue with putting the opening brace on the next line. Does it break anything else apart from returning object literals?

I do not have a fully deep enough knowledge of the various JavaScript parsers to confirm that or not.
What I can say is that using the JavaScript style for braces guarantees that you will never have to face that problem.

I know that you’re used to one style in another programming language. You’re programming in JavaScript now.
It’s time for you to write your code in the language that you’re writing it in.

I know that I’m programming in JavaScript. People that don’t know JavaScript use jQuery snippets they’ve copied and pasted from various sites. People that do know JavaScript use more advanced libraries (MooTools, Dojo) or write their own. I don’t think a relatively small styling choice affects someone’s knowledge of a language. There’s no “one true style” as there’s no “official” JavaScript style guidelines. I like seeing the braces line up.

Your chosen style of putting braces on the left is pulled in from other programming languages is directly responsible for allowing automatic-semicolon-insertion bugs.
I am saying that putting braces on the right is the style that must be used to protect JavaScript code from ever having that problem.

I am saying that putting braces on the right is the style that must be used to protect JavaScript code from ever having that problem.

And I’m saying that I’ve never had an issue with my coding style so I’m not going to change it just to avoid a single issue that I almost never encounter. On the rare chance that I encounter an automatic-semicolon-insertion bug, I can generally tell that that’s the issue (and that’s what unit tests are for, to catch potential issues like that).

We’ll have to agree to disagree :slight_smile:

We will indeed. You can continue to program in a style that you have brought in from other programming languages, and I will continue to program in a style that is sympathetic to the unique issues of JavaScript.

Wow, Paul you’ve been extraordinarily unhelpful, curly brace positioning wasn’t the question…

Nothing stands out in your code as wrong to me, though I don’t personally use this type of class instantiation in my js.
Most of the time I find maintaining state in one object easier to work with and then use a prototype when there needs to be more than one. I’ve never used super classes in js or extending objects. so if I were to make a class implementation I would only make a wrapper around init() and Object.create because that’s all I use it for.

But, if you prefer coding in a more OO way there’s nothing wrong with that.

You can continue to program in a style that you have brought in from other programming languages

What if I said I learned JavaScript first? (which I didn’t, but still) Not every JavaScript guide uses the K&R brace style. Just because your opinion is that it’s better, you can’t say that everyone else in the world thinks the same way, nor can you say it’s the “right” way. With something subjective like that, there is no “right way”.

Nothing stands out in your code as wrong to me, though I don’t personally use this type of class instantiation in my js.

Yeah, some people use it while others don’t. I’ve found it handy when controls are fully self-contained (like the autocomplete on http://atarcalc.com/) but there’s other times where I’ve just used object literals.

I’ve never used super classes in js or extending objects

I don’t use them too often either. One example I could think of was a drawing app - Drawing tools could inherit from a base Tool class that would implement basic behaviour shared by all the tools. Some scenarios can also be solved using composition instead of inheritance, so I’m planning to add some sort of mixin support as well.

I think prototypes are definitely necessary if you have hundreds of controls on a page (for whatever reason) and need to maintain state for all of them.