CODEWITHSUNDEEP
javascriptjqueryoptimization
Zach Both
Zach Both

Reputation: 119

How do I make this jquery code less redundant and more efficient?

I'm relatively new to writing javascript. The js code below is modified from a previous answer I found on here. It does exactly what I want it to do functionality-wise, however, it's redundant and calling the code every time the mouse enters (which takes more resources than it needs too).

Any suggestions on how to make this less redundant and more efficient?

SIMPLIFIED DEMO HERE: http://jsfiddle.net/nsnzd9cL/

HTML:

<div class="container">
    <div class="category" id="commercials">
        <p>Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />
        </p>
        <div class="scroll">
            <p>Scroll</p>
        </div>
    </div>
    <div class="category" id="photography">
        <p>Something
            <br />Something
            <br />
        </p>
        <div class="scroll">
            <p>Scroll</p>
        </div>
    </div>
    <div class="category" id="experiments">
        <p>Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />Something
            <br />
        </p>
        <div class="scroll">
            <p>Scroll</p>
        </div>
    </div>
</div>

JS:

  var arrow1 = $('#commercials .scroll');
var arrow2 = $('#photography .scroll');
var arrow3 = $('#experiments .scroll');

$("#commercials").mouseenter(function () {

    if ($('#commercials').hasScrollBar()) {
        arrow1.css({
            'visibility': 'visible'
        });
    } else {
        arrow1.css({
            'visibility': 'hidden'
        });
    }
});

$("#photography").mouseenter(function () {

    if ($('#photography').hasScrollBar()) {
        arrow2.css({
            'visibility': 'visible'
        });
    } else {
        arrow2.css({
            'visibility': 'hidden'
        });
    }
});

$("#experiments").mouseenter(function () {

    if ($('#experiments').hasScrollBar()) {
        arrow3.css({
            'visibility': 'visible'
        });
    } else {
        arrow3.css({
            'visibility': 'hidden'
        });
    }
});

(function ($) {
    $.fn.hasScrollBar = function () {
        return this.get(0).scrollHeight > (this.height() + 1);
    }
})(jQuery);

Upvotes: 1

Views: 101

Answers (5)

Eric
Eric

Reputation: 97571

Continueing from Vijay's answer, we can combine the selector:

(function ($) {
    $.fn.hasScrollBar = function () {
        return this.get(0).scrollHeight > (this.height() + 1);
    }

    $("#commercials,#photography,#experiments").on('mouseenter', function() {
        var arrow = $(this).find('.scroll');

        if ( $(this).hasScrollBar() ) {
            arrow.css({
                'visibility': 'visible'
            });
        } else {
            arrow.css({
                'visibility': 'hidden'
            });
        };
    };
})(jQuery);

Upvotes: 0

Rahul
Rahul

Reputation: 334

Easy to read and non redundant code:

var arrow1 = $('#commercials .scroll');
var arrow2 = $('#photography .scroll');
var arrow3 = $('#experiments .scroll');

function bindMouseEnter(id, arrow){

   $("#"+id).mouseenter(function () {

       if ($('#'+id).hasScrollBar()) {
          arrow.css({
            'visibility': 'visible'
          });
      } else {
        arrow.css({
            'visibility': 'hidden'
        });
      }
    });

}

bindMouseEnter('commercials', arrow1);
bindMouseEnter('photography', arrow2);
bindMouseEnter('experiments', arrow3);


(function ($) {
    $.fn.hasScrollBar = function () {
        return this.get(0).scrollHeight > (this.height() + 1);
    }
})(jQuery);

Upvotes: 0

vol7ron
vol7ron

Reputation: 42099

JSFiddle

Bind the event to the container and apply it to the elements within the container, in this case the specified IDs. Then look up elements relative to the target when triggered:

$('.container').on('mouseenter','#commercials,#photography,#experiments', function(){
    var $this   = $(this),
        $scroll = $this.find('.scroll');

    if( $this.hasScrollBar() ){
        $scroll.css('visibility','visible');
    } else {
        $scroll.css('visibility','hidden');
    }   
});

(function ($) {
    $.fn.hasScrollBar = function () {
        return this.get(0).scrollHeight > (this.height() + 1);
    }
})(jQuery);

but really, I'd replace the '#commercials,#photography,#experiments' with '.category'; JSFiddle using .category.

Using .category gives you the added benefit of adding new categories dynamically and not having to rebind events when they're created after the page load, since the event is still on the container.

Upvotes: 1

Ben
Ben

Reputation: 1493

This is how I would do it...

JS Fiddle

$(document).ready(function(){
    scrollCheck();
});

function scrollCheck() {
    var div = '',
        scroll = true;
    $('.category').each(function( index ) {
        div = $(this);
        scroll = div.get(0).scrollHeight <= (div.height() + 1);
        console.log(scroll); // just a check
        if (scroll) {
            div.children('.scroll').remove();
        }
    });
}

Upvotes: 0

Vijay Dev
Vijay Dev

Reputation: 1114

I have reduce all the redundancy I could find: 

(function ($) {
    $.fn.hasScrollBar = function () {
        return this.get(0).scrollHeight > (this.height() + 1);
    }

    var $commercials = $( "#commercials" ),
        $photography = $( "#photography" ),
        $experiments = $( "#experiments" );

    var mouseEnterListner = function() {
        var arrow = $(this).find( '.scroll' );

        if ( $(this).hasScrollBar() ) {
            arrow.css({
                'visibility': 'visible'
            });
        } else {
            arrow.css({
                'visibility': 'hidden'
            });
        };
    };

    $commercials.on('mouseenter', function() {
        mouseEnterListner.call(this);
    });

    $photography.on('mouseenter', function() {
        mouseEnterListner.call(this);
    });

    $experiments.on('mouseenter', function() {
        mouseEnterListner.call(this);
    });
})(jQuery);

Good luck, have fun!

Upvotes: 0

Related Questions