Skip to main content
added 487 characters in body
Source Link
James Khoury
  • 3.2k
  • 1
  • 25
  • 51

You ask:

  1. The tooltip accepts "settings" options which I extend the standard jQuery way. Should I be passing only the necessary properties into the "Tip" object for each tooltip rather than the whole settings object, which right now I'm just doing out of convenience and simplicity?

A:

No. If you filter out any "extra" settings and someone comes along to extend your code they will have trouble if they want to add extra settings. There doesn't seem to be any reason to remove them.

Also on this point

settings = $.extend(defaultSettings,settings);

is normally

settings = $.extend({}, defaultSettings, settings || {});

This allows settings to be optional and defaultSettings won't be overridden.

I would also move var defaultSettings outside your $.fn.wTip function (but still in your closure (function($){) this will mean its not created every time.

While it is not necessary, I would return this in your eventScheduler class

function eventScheduler()
{
    return this;
}  

Its easier to understand that it is a class. (Also most class are Capitalized but thats just a style).

Overall I would say you have a nice clean coding style.

You ask:

  1. The tooltip accepts "settings" options which I extend the standard jQuery way. Should I be passing only the necessary properties into the "Tip" object for each tooltip rather than the whole settings object, which right now I'm just doing out of convenience and simplicity?

A:

No. If you filter out any "extra" settings and someone comes along to extend your code they will have trouble if they want to add extra settings. There doesn't seem to be any reason to remove them.

Also on this point

settings = $.extend(defaultSettings,settings);

is normally

settings = $.extend({}, defaultSettings, settings || {});

This allows settings to be optional and defaultSettings won't be overridden.

You ask:

  1. The tooltip accepts "settings" options which I extend the standard jQuery way. Should I be passing only the necessary properties into the "Tip" object for each tooltip rather than the whole settings object, which right now I'm just doing out of convenience and simplicity?

A:

No. If you filter out any "extra" settings and someone comes along to extend your code they will have trouble if they want to add extra settings. There doesn't seem to be any reason to remove them.

Also on this point

settings = $.extend(defaultSettings,settings);

is normally

settings = $.extend({}, defaultSettings, settings || {});

This allows settings to be optional and defaultSettings won't be overridden.

I would also move var defaultSettings outside your $.fn.wTip function (but still in your closure (function($){) this will mean its not created every time.

While it is not necessary, I would return this in your eventScheduler class

function eventScheduler()
{
    return this;
}  

Its easier to understand that it is a class. (Also most class are Capitalized but thats just a style).

Overall I would say you have a nice clean coding style.

Source Link
James Khoury
  • 3.2k
  • 1
  • 25
  • 51

You ask:

  1. The tooltip accepts "settings" options which I extend the standard jQuery way. Should I be passing only the necessary properties into the "Tip" object for each tooltip rather than the whole settings object, which right now I'm just doing out of convenience and simplicity?

A:

No. If you filter out any "extra" settings and someone comes along to extend your code they will have trouble if they want to add extra settings. There doesn't seem to be any reason to remove them.

Also on this point

settings = $.extend(defaultSettings,settings);

is normally

settings = $.extend({}, defaultSettings, settings || {});

This allows settings to be optional and defaultSettings won't be overridden.