Making my JavaScript/jQuery cleaner

Question

I'm trying to improve how clean my JavaScript/jQuery is and was wondering if anyone has any pointers.

When I look at this is just doesn't look clean...

if (window.jQuery) {
    (function ($) { 

        var podCaption = function ($scope, settings) { 

            this._settings = $.extend({  
                openHeight : 75,
                expandHeight : 120,
                shrinkHeight : 30,
                closeHeight : 15,
                timer : ''
            }, settings); 
            this._elements = { 
                scope : $scope,
                caption : $('.slider-information', $scope)
            };  

            this.init();
        };

        podCaption.prototype.init = function() {
            var _this = this; 

            $('.photo-more', _this._elements.caption).live('click', function() {  
                _this.expand(_this);
            });

            _this._elements.caption.mouseenter(function() {  
                _this.open(_this);
            }).mouseleave(function() {
                _this._settings.timer = setTimeout(function() { 
                    _this.shrink(_this);
                }, 1000);
            }); 
        };

        podCaption.prototype.changeImage = function(photoIndex, image) {
            var _this = this; 

            //Shrink out content 
            _this.close(_this, function() { 
                //Build content - NOTE i'm actually doing some template stuff here but I'm trying to make the code a little less verbose for the question at hand 
                _this._elements.caption.empty();
                _this._elements.caption.append('<div><div class="photo-description">..</div><div class="photo-more">...</div><div class="photo-info">...</div></div>'); 

                _this.open(_this, function() {   
                    _this._settings.timer = setTimeout(function() { 
                        _this.shrink(_this);
                    }, 4500);
                });
            }); 
        }; 

        podCaption.prototype.expand = function(_this, callback) {
            clearTimeout(_this._settings.timer);

            var caption = _this._elements.caption;
            $('.photo-more', caption).hide();
            $('.photo-info', caption).fadeIn(); 
            caption.animate({ height : _this._setting.expandHeight, opacity : 0.8 }, 500, callback);
        }

        podCaption.prototype.open = function(_this, callback) {
            clearTimeout(_this._settings.timer);

            _this._elements.caption.animate({ height : _this._setting.openHeight, opacity : 0.8 }, 500, callback);
        }

        podCaption.prototype.close = function(_this, callback) {
            clearTimeout(_this._settings.timer);

            var caption = _this._elements.caption;
            caption.children().fadeOut();
            caption.animate({ height : _this._setting.closeHeight, opacity : 0.2 }, 400, callback);
        }

        podCaption.prototype.shrink = function(_this, callback) {
            clearTimeout(_this._settings.timer);

            var caption = _this._elements.caption;
            $('.photo-info', caption).fadeOut(function() { $('.photo-more', caption).show(); });
            caption.animate({ height : _this._setting.shrinkHeight, opacity : 0.3 }, 400, callback);
        }

        $.fn.podCaption = function (options) {
            return new podCaption(this, options);
        };  
    })(jQuery);
}

Its a pod that shows at the bottom of a photo that opens and closes. It has a couple of different states as you can see and it is designed to show different content when the photos changes - hence changeImage. Does that help? — vdh_ant
– vdh_ant, Commented Feb 19, 2011 at 1:07
This is a question better suited to codereview.stackexchange.com. — Matt Ball
– Matt Ball, Commented Feb 19, 2011 at 1:17

Šime Vidas · Accepted Answer · 2011-02-19 01:40:16Z

1

Instead of this:

if (window.jQuery) {
    (function ($) {     
        // code     
    })(jQuery);
}

how about this:

(function($) {
    if ( !$ ) return;
    // code
}(jQuery));

Also, this:

_this._elements.caption.empty();
_this._elements.caption.append('<div>...');

can be chained:

_this._elements.caption.empty().append('<div>...');

or just this:

_this._elements.caption.html('<div>...');

Also, you could define the prototype in object literal form:

podCaption.prototype = {
    expand: function(_this, callback) {
        // code
    },
    changeImage: function(photoIndex, image) {
        // code
    },
    ...
};

edited Feb 19, 2011 at 1:40

answered Feb 19, 2011 at 1:31

Šime Vidas

187k66 gold badges290 silver badges392 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Raynos Over a year ago

There's a dicussion here. Apparently blinding following a single return point is considered bad practice. So that particular case is fine.

island205 · Accepted Answer · 2011-02-19 02:13:31Z

window.jQuery && (function($){
    var podCaption = function ($scope, settings) {

        this._settings = $.extend({
            openHeight : 75,
            expandHeight : 120,
            shrinkHeight : 30,
            closeHeight : 15,
            timer : ''
        }, settings);
        this._elements = {
            scope : $scope,
            caption : $('.slider-information', $scope)
        };

        this.init();
    };
    $.extend(podCaption.prototype,{
        init:function() {
            var that = this;

            $('.photo-more', this._elements.caption).live('click', function() {
                that.expand();
            });

            this._elements.caption.mouseenter(function() {
                that.open();
            }).mouseleave(function() {
                that._settings.timer = setTimeout(function() {
                    that.shrink();
                }, 1000);
            });
        },
        changeImage:function(photoIndex, image) {
            var that = this;

            //Shrink out content
            this.close(function() {
                //Build content - NOTE i'm actually doing some template stuff here but I'm trying to make the code a little less verbose for the question at hand
                that._elements.caption.empty();
                that._elements.caption.append('<div><div class="photo-description">..</div><div class="photo-more">...</div><div class="photo-info">...</div></div>');

                that.open(function() {
                    that._settings.timer = setTimeout(function() {
                        that.shrink();
                    }, 4500);
                });
            });
        },
        expand:function(callback) {
            clearTimeout(this._settings.timer);

            var caption = this._elements.caption;
            $('.photo-more', caption).hide();
            $('.photo-info', caption).fadeIn();
            caption.animate({
                height : this._setting.expandHeight,
                opacity : 0.8
            }, 500, callback);
        },
        open:function(callback) {
            clearTimeout(this._settings.timer);

            this._elements.caption.animate({
                height : this._setting.openHeight,
                opacity : 0.8
            }, 500, callback);
        },

        close:function(callback) {
            clearTimeout(this._settings.timer);

            var caption = this._elements.caption;
            caption.children().fadeOut();
            caption.animate({
                height : this._setting.closeHeight,
                opacity : 0.2
            }, 400, callback);
        },

        shrink:function(callback) {
            clearTimeout(this._settings.timer);

            var caption = this._elements.caption;
            $('.photo-info', caption).fadeOut(function() {
                $('.photo-more', caption).show();
            });
            caption.animate({
                height : this._setting.shrinkHeight,
                opacity : 0.3
            }, 400, callback);
        }
    });

    $.fn.podCaption = function (options) {
        return new podCaption(this, options);
    };
})(window.jQuery);

maybe like this. main change:extend prototype,this scope,out if statement

Hirvesh · Accepted Answer · 2011-02-19 04:46:42Z

0

Try using JSLint http://www.jslint.com/ to prettify the code if that's what you what...

answered Feb 19, 2011 at 4:46

Hirvesh

8,01216 gold badges62 silver badges74 bronze badges

Collectives™ on Stack Overflow

Making my JavaScript/jQuery cleaner

3 Answers 3

1 Comment

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related