CODEWITHSUNDEEP
javascriptjquery
user2168066
user2168066

Reputation: 635

Need query to be constructed only once

I have a function that uses .slidetoggle to collapse 2 paragraphs, and create a div to "expand". The code is working correctly but I am trying to optimize it to where I don't need to construct the query each time. any suggestions?

    <script type="text/javascript">
    $(document).ready(function(){

        $(".press_release").each(function(){
            if($(this).children("p").length > 1){
                $('<div><a href="#readmore" class="readmore">Read More&hellip;</a></div>').appendTo($(this));

                $(this).children("p").first().addClass('first-p');
                $(this).children("p").not('.first-p').hide();

                $(this).find('.readmore').click(function(){
                    $(this).parent().siblings('p').not('.first-p').slideToggle(500);
                    return false;
                });
            }

        });
    });
</script>

Upvotes: 0

Views: 38

Answers (2)

Brandon Gano
Brandon Gano

Reputation: 6720

As TGH mentions, you should cache any repeated queries. In your code, there are three main queries we care about: the individual press release ($(this)), the first paragraph, and the rest of the paragraphs.

$(document).ready(function(){

  $(".press_release").each(function(){
    var $this = $(this),
      $paragraphs = $this.children("p"),
      $firstParagraph = $paragraphs.first();

    // Remove the first paragraph from our set
    $paragraphs = $paragraphs.not($firstParagraph);

    // We're only counting paragraphs after the first
    if($paragraphs.length > 0){
      $('<div><a href="#readmore" class="readmore">Read More&hellip;</a></div>').appendTo($this);

      $firstParagraph.addClass('first-p');
      $paragraphs.hide();

      // Delay the lookup of .readmore now; gain a slight benefit here at the
      // expense of a tiny cost each time something inside $this is clicked.
      // Binding directly to the .readmore element as you've done in your
      // original code likely won't have any noticeable impact either way.
      $this.on('click', '.readmore', function (){
        $paragraphs.slideToggle(500);
        return false;
      });
    }

  });
});

Upvotes: 0

TGH
TGH

Reputation: 39268

I would cache the $(this) reference in order to avoid recreating the jquery object over and over again:

$(".press_release").each(function(){
       var myThis = $(this); 
       if(myThis.children("p").length > 1){
  ....

Use the cached reference throughout the script.

Upvotes: 2

Related Questions