CODEWITHSUNDEEP
javascriptjqueryfunction
wetjosh
wetjosh

Reputation: 6398

How to turn this messy block of code into a function

I have this block of code that works just as I want it to but I want to turn it into a function and reuse it here and there. Below is the working code followed by my conversion into a function. But I'm doing something wrong cause it doesn't work.

Working

$('div.destination a').click(function(){
    $('html, body').stop(true,false).animate({
        scrollLeft: $( $.attr(this, 'href') ).offset().left 
                        - 1/2 * $(window).width() 
                        + 1/2 * $( $.attr(this, 'href') ).width()
    },
    {
        duration: (Math.abs( $( $.attr(this, 'href') ).offset().left 
                    - $(document).scrollLeft() )) 
                    / 1000 
                    / spaceScaleFactor 
                    / travelRate,
        easing: 'linear',
        queue: false
    });
    $('div.destination').prev().children().children().text(($.attr(this, 'href')).substring(1));
    return false;
});

Function and calling it when clicked (not working)

// Make the function and replace 'this' with 'x'

function travelToDestination (x) {
        $('html, body').stop(true,false).animate({
            scrollLeft: $( $.attr(x, 'href') ).offset().left 
                            - 1/2 * $(window).width() 
                            + 1/2 * $( $.attr(x, 'href') ).width()
        },
        {
            duration: (Math.abs( $( $.attr(x, 'href') ).offset().left 
                        - $(document).scrollLeft() )) 
                        / 1000 
                        / spaceScaleFactor 
                        / travelRate,
            easing: 'linear',
            queue: false
        });
        $('div.destination').prev().children().children().text(($.attr(x, 'href')).substring(1));
        return false;
    });
}

//call the function on click using '$(this)' as a parameter

$('div.destination a').click(function(){
    travelToDestination($(this));
}

Like I said, the code works fine as it is. I just want to know what I am doing wrong when I try and make it a function. It may be that 'this' does not equal '$(this)'. Thanks!

Upvotes: 1

Views: 115

Answers (2)

jcolebrand
jcolebrand

Reputation: 16025

Try this change:

$('div.destination a').click(travelToDestination);

and change all references to x back to this. This way the this in your function is the button you clicked. At that point, what you have defined as x is the event of the click function, not the this element you had previously.

What you're doing when you do this is passing a function reference to the click handler, which is the same as passing the function itself. So instead of 

$(selector).click(function _travelToDestination_inlineFunction(){ /* stuff */ })

you're just telling it to go find the function and use that. Also note the use of the named function in my "anonymous function" above. That always helps me with stack debugging later "where did I call that from again ..".

Second thing you did wrong: In the first one you're passing a this and in the second one you're passing a $(this) which you could bypass in the latter function (and leaving everything else alone) by adding this line:

function travelToDestination (me) {
    x = me[0]; //I added this line
    $('html, body').stop(true,false).animate({

but I wouldn't do that. I would just alter the first one.

Upvotes: 0

Barmar
Barmar

Reputation: 780861

Change it to:

$('div.destination a').click(function(){
    travelToDestination(this);
}

Since you replaced this with x, it expects the argument to be the DOM element, not wrapped in a jQuery object.

Upvotes: 4

Related Questions