Am I on the right track with JavaScript/jQuery?

I've been trying to learn how to develop jQuery plugins but have little guidance on the matter. I see lots of code on the web, of course, but am not sure what exactly constitutes best practices due to the level of variability in style.

-
$.notify - Displays a simple notification bar similar to that of WP7's notification bar.

-
$.every - Calls a callback/handler every n-seconds with an additional delay (used for timing animations)

```
(function ($) {
$.notify = function (message, options) {
var bodyElement = $('body');

var controller = bodyElement.data('jscom.NotifyController');
if(controller == undefined || controller == null){
controller = new NotifyController(options);
bodyElement.data('jscom.NotifyController', controller);
}

controller.notify(message, options);

return controller;
};

function NotifyController(options){
// Helper elements & variables
var bodyElement = $('body');
var scope = this;

var notificationElement = bodyElement.find('#JSNotification');

if(notificationElement == undefined || notificationElement.length ');
notificationElement = bodyElement.find('#JSNotification');
notificationElement.html('');
}

this.updateSettings(options);

this.target = notificationElement;
this.timer = null;
this.timestamp = 0;

this.target.hover(
function() { $(this).addClass('mouse-over'); },
function() { $(this).removeClass('mouse-over'); scope.onMouseOut(); }
);

this.target.every(1, 0, function(){
scope.update();
});
};

NotifyController.prototype.updateSettings = function(options){
/ Setup the settings & options /
var defaults = { timeout: 4000, cssClass: 'default' };
var settings = $.extend(
{ },
defaults,
options

OK, let's see.

First of all by glancing on the code, you don't seem to know what == does.

Especially by looking at this.timer != undefined && this.timer != null this does the exact same check twice as type coercion is happening behind the scenes. Type coercion also makes 0 == '' // true. You should read up on it.

No, concerning the style, I took the time to refactor your code a bit (while hopefully not introducing any errors :D )

```
(function ($) {

$.notify = function(message, options) {

var bodyElement = $('body'),
controller = bodyElement.data('jscom.NotifyController');

if (controller == null) { // using type coercion is fine, but only for a null and undefined check at the same time
controller = new NotifyController(options);
bodyElement.data('jscom.NotifyController', controller);
}

controller.notify(message, options);
return controller;

};

function NotifyController(options) { // Whitespace before the {

// Helper elements & variables
var bodyElement = $('body'),
that = this; // "scope" here well, most people use "that". There are some people that use "self", but keep in mind that there is a global variable called "self" that references the window object.

var notificationElement = bodyElement.find('#JSNotification');

if (notificationElement === undefined || notificationElement.length ');
notificationElement = bodyElement.find('#JSNotification');
notificationElement.html('');
}

this.updateSettings(options);

this.target = notificationElement;
this.timer = null;
this.timestamp = 0;

// you should avoid putting stuff on the same line
this.target.hover(function() {
$(this).addClass('mouse-over');

}, function() {
$(this).removeClass('mouse-over');
that.onMouseOut();
}
);

this.target.every(1, 0, function() {
that.update();
});

};

// Doing NotifyController.prototype.updateSettings = .. is cumbersome and bloated, just assign the prototype object
// The style below is a lot more readable
NotifyController.prototype = {

updateSettings: function(options) {
/ Setup the settings & options /
var defaults = { // again avoid stuff on the same line
timeout: 4000,
cssClass: 'default'
};

// no need here though
var settings = $.extend({}, defaults, options);
this.timeout = settings.timeout;
this.cssClass = settings.cssClass;

},

notify: function(message, options){

if (this.timer != null) {
clearTimeout(this.timer);
this.timer = null;
}

this.target.attr('class', '');
this.updateSettings(options);

var timestamp = new Date();

this.target.find('p').html(
'' +
timestamp.format("h:MM:ss TT").toString() +
'' +
message +
''
)

this.timestamp = timestamp;
this.target.addClass('active').addClass(this.cssClass);

var that = this;
this.timer = setTimeout(function() {
that.close();

}, this.timeout);

},

update: function() {

if (this.target.hasClass('mouse-over')) {
this.timestamp = new Date();
return;
}

var time = new Date();
var delta = time - this.timestamp;

var percent = (delta / this.timeout * 100).toFixed(0);

// Also stay away from shorthand ifs, not a good style to go with.
if (percent > 100) {
percent = 100;
}

this.target.find('.progress').css('width', percent.toString() + '%');
},

onMouseOut: function() {

if (this.timer != undefined && this.timer != null) {
clearTimeout(this.timer);
this.timer = null;
}

var that = this;
this.timer = setTimeout(
function() { that.close(); },
this.timeout
);

this.timestamp = new Date();
};

close: function() {

// Whitespace on ifs also helps readability
// it also aligns the body with the condition :)
if (this.target.hasClass('mouse-over')){
return;
}

this.target.removeClass('active'); //.removeClass(this.cssClass);

}

}

/ EVERY CONTROLLER /
$.fn.every = function(interval, pauseInterval, callback, id) {

if (id != null) {
id = '';
}

var controller =

StackExchange Code Review Q#8754, answer score: 2