What's Wrong With This Code? (#28)

Victor Brumble is writing code again, and this time he's writing a jQuery plugin. His plugin makes alert boxes easy, because Victor loves pointless, modal dialogs.

(function ($) {

    var settings = {
        text: 'Thank you for clicking!'
    };
    
    $.fn.alerter = function (options) {

        return this.each(function () {

            if (options) {
                $.extend(settings, options);
            }

            $(this).click(function () {
                alert(settings.text);
            });
        });
    };
})(jQuery);

As you can see, Victor has a default text message associated with his plugin, but he also learned how to use jQuery.extend to allow callers to override the default text. The plugin is working well for Victor, but another developer is complaining about the following code not working correctly.

<p id="p1">
    This is paragraph 1.
</p>
<p id="p2">
    This is paragraph 2.
</p>
<script type="text/javascript">
    $("#p1").alerter({ text: "Please don't click again!" });
    $("#p2").alerter({ text: "This is not the text you want!" });
</script>

What could possibly be wrong?

Print | posted @ Monday, August 15, 2011 9:12 AM

Comments on this entry:

Gravatar # re: What's Wrong With This Code? (#28)
by Mike at 8/15/2011 9:41 AM

the settings object is going to be shared between the two instances since it is declared outside the each().
  
Gravatar # re: What's Wrong With This Code? (#28)
by Kenneth at 8/15/2011 9:47 AM

$.fn.alerter = function (options) {

return this.each(function () {

var newSettings = $.extend({}, settings, options);

$(this).click(function () {
alert(newSettings.text);
});
});
};

  
Gravatar # re: What's Wrong With This Code? (#28)
by Peter Bromberg at 8/15/2011 11:40 AM

Victor needs some tutelage in grammar too. It's "MODAL", not "MODEL" dialogs that he likes.
  
Gravatar # re: What's Wrong With This Code? (#28)
by Brent Anderson at 8/15/2011 11:55 AM

(function ($) {
$.fn.alerter = function (options) {
var settings = {
text: 'Thank you for clicking!'
};
if (options) {
$.extend(settings, options);
}
return this.each(function (index,item) {
$(item).click(function () {
alert(settings.text);
});
});
};
})(jQuery);
  
Gravatar # re: What's Wrong With This Code? (#28)
by scott at 8/15/2011 11:56 AM

@PB - thanks for catching that :)
  
Gravatar # re: What's Wrong With This Code? (#28)
by Peter Bromberg at 8/15/2011 12:09 PM

Mark Bennett made a jsFiddle of this.
http://jsfiddle.net/d7WHx/2/
  
Gravatar # re: What's Wrong With This Code? (#28)
by Harry at 8/16/2011 12:38 AM

Mike and Kenneth are right. The setting variable is shared and overwritten.

http://jsfiddle.net/harrychou/Lhj49/
  
Gravatar # re: What's Wrong With This Code? (#28)
by Harry at 8/16/2011 12:54 AM

http://jsfiddle.net/harrychou/Lhj49/3/
  
Gravatar # re: What's Wrong With This Code? (#28)
by Duncan Smart at 8/16/2011 6:27 AM

Should `this.each` be `$(this).each`?
  
Gravatar # re: What's Wrong With This Code? (#28)
by Duncan Smart at 8/16/2011 6:32 AM

Nope, scratch that :-) `this` is a jQuery object in a plugin (at that initial scope)...
  
Gravatar # re: What's Wrong With This Code? (#28)
by kipper mcslice at 9/5/2011 12:06 AM

#{aka (...)} t,"" dun duda duh
  
Comments have been closed on this topic.
Scott Allen
Posts - 869
Comments - 4493
Stories - 14