Hi Ben,
nice to see you on this list!
Thanks for the feedback. I wasn't sure if the THIS scope was bad to
use since it pointed to the jQuery object itself. But I guess, the
whole point of the plugin is that you are extending the very nature of
the jQuery object.
I haven't seen that approach before, so basically I'd say that approach
may yield some interesting results, or it doesn't bring any benefit in
contrast to just defining plugin-function local functions. In code,
something like this instead:
$.fn.plugin = function() {
function handler() {
...
}
$("...").bind("...", handler);
};
Binding the handler to the local scope increases its visibility a lot.
That could be handy at some point where you want to access it. But as
there is no need for it, you shouldn't do it. Adding to the local scope
could overwrite a prototype method with the event handler. Imagine a
handler-plugin:
$.fn.handler = function() { ... };
Now adding to this could actually have a bad effect:
$.fn.plugin = function() {
this.handler = function() {
...
}
$("...").bind("...", handler);
};
Because you are overwriting another method. $("...").plugin().handler()
wouldn't yield the expected result.
In addition, instead of defining an event handler and calling that
inside an anonymous function, you could just as well call the handler
directly. Inside all jQuery-event-handlers, "this" points to the
event-handling object, so inside your OnMouseOver you could replace
"jNode" with "this".
I'm a fan of writing simple code instead of numerous comments. Its a
win-win when you can replace a comment with a good method name that
explains the purpose and improves the code. The validator-object I use
within the validation plugin has over 30 methods, a lot of those only
containing one line. That helps a lot when extending the code, because
for almost everything there is a little method available that does just
what you need, no worries about copying code.
Another example of simplifying:
// Return the THIS pointer such that the jQuery chain
// is not broken.
return( this );
->
// don't break the chain
return this;
The switch could be simplified a lot by switching (sorry) to a map:
var directions = {
"0": { "top": intTop - intDistance }
...
}
jNode.css(directions[intDirection]);
You wouldn't even need a variable for that object (Untested, may be
screwed):
jNode.css({
"0": { "top": intTop - intDistance },
...
}[intDirection]);
Like written before, please add an alert to those buttons on click.
Makes that plugin a cool game. Add a hitcount and highscore and you're done.
--
Jörn Zaefferer
http://bassistance.de
