Show and hide posts using setTimeout

Question

I have made an effect that has the h1 move and then reveals p using .addClass, .removeClass and CSS3 transition. It works perfectly but I think I am repeating too much of the code and I think there may be a way to write it better. I need postShow and postHide to be different because I need setTimeout to be in postShow for timing. Please review the code below and give me any pointers.

jQuery(document).ready(function($){

    var post          = $('div.post');

    var postShow = function(item) {
            $(item).closest('a').find('img.postimg').addClass('imggrow');
            $(item).closest('a').find('h1').addClass('bannerhover');
            setTimeout(function(){
                $(item).closest('a').find('p.excerpt').addClass('phover');
            },500);
        };

    var postHide = function(item) {
            $(item).closest('a').find('img.postimg').removeClass('imggrow');
            $(item).closest('a').find('h1').removeClass('bannerhover');
            $(item).closest('a').find('p.excerpt').removeClass('phover');
    };

//-----------------------------------------------------------

    if ($(window).width() > 1000) {

        post.hover(function() {
            $(this).find('a').find('p.excerpt').show(0, function() {
                postShow(this);
            });
        },function() {
            $(this).find('a').find('p.excerpt').hide(0, function() {
                postHide(this);
            });
        });

    }

//--------------------------------
}); 

Answer 1

Forewarning: My first post in Code Review! Here goes.

jQuery(document).ready(function($){

var post = $('div.post');

function postShowHide(showing) {
        var a = $(this).closest('a');
        a.find('img.postimg').toggleClass('imggrow', showing);
        a.find('h1').toggleClass('bannerhover', showing);
        function sethover(){
            a.find('p.excerpt').toggleClass('phover', showing);
        }
        if (showing) {
            setTimeout(sethover, 500);
        }
        else {
            sethover();
        }
    };

//-----------------------------------------------------------

if ($(window).width() > 1000) {

    post.hover(function() {
        $(this).find('a p.excerpt').show(0, postShowHide.bind(this, true));
    },function() {
        $(this).find('a p.excerpt').hide(0, postShowHide.bind(this, false));
    });

}

//--------------------------------
});

Couple things I made use of here: JQuery and similar frameworks have a toggle class variant that takes a Boolean parameter to either add or remove the class. This is useful for any situation that adds/removes the class since it means you only need one function, not two (as long as you can take a parameter)

To make the setTimeout(500) part work with that, I did end up separating it into a function, and conditional calling mechanism. This at least centralizes the logic, although it does elongate the method a bit.

If you're creating a callback that simply calls another function, often you can use Function.bind instead. The first argument is the target value of the this keyword, then the following arguments will be passed into the function. (If you're me and have an arbitrary contractual obligation to support Safari 5.0 for no reason, you can't use it but JQuery may have a similar method.)

Answer 2

Firstly, you should add a class to the a element instead, since that's what all the other elements are contained in.

That way, you don't have to add/remove classes on all the child elements separately. You can just add/remove 1 class, and set up your CSS to style the child elements accordingly.

You also decouple your JS from the HTML to greater extent; now you can control things in just the CSS instead of having the HTML, the CSS, and the JS all concern themselves with the exact structure of your page.

Given your current code, however, I would suggest something like this:

jQuery(function($) { // shortcut for $(document).ready(...)
  function showPost() {
    var a = $(this).closest("a"); // get this once, use it thrice
    a.find('img.postimg').addClass('imggrow');
    a.find('h1').addClass('bannerhover');
    a.find('p.excerpt').delay(500).addClass('phover'); // note the delay() call
  }

  function hidePost() {
    var a = $(this).closest("a"); // same as above
    a.find('img.postimg').removeClass('imggrow');
    a.find('h1').removeClass('bannerhover');
    a.find('p.excerpt').removeClass('phover');
  }

  if ($(window).width() > 1000) {
    // No need to use hide/show with a timeout of zero, as far as I can tell.
    // Just pass the two functions directly
    $('div.post').hover(showPost, hidePost);
  }
});