Refactoring jQuery code into reusable function

Question

I would really appreciate it if someone could help me refactor this code into a reusable function and show me how to use it. I'm just a beginner, but can see that I'm repeating lots of stuff and I'm sure there's a way to write it better.

I can add html if it's needed.

$("[value=home]").click(function() {
    $(currentLink).removeClass("cantclick");
    $(current).hide();
    $(this).addClass("cantclick");
    $("#texts").fadeIn(900);
    current = $("#texts");
    currentLink = $("[value=home]");
    console.log("current is " + current.text() + ".");
});

$("[value=aboutme]").click(function() {
    $(currentLink).removeClass("cantclick");
    $(current).hide();
    $(this).addClass("cantclick")
    $("#aboutmetext").fadeIn(900);
    current = $("#aboutmetext");
    currentLink = $("[value=aboutme]");
    console.log("current is " + current.text() + ".");
});

$("[value=tuition]").click(function() {
    $(currentLink).removeClass("cantclick");
    $(current).hide();
    $(this).addClass("cantclick")
    $("#tuitiontext").fadeIn(900);
    current = $("#tuitiontext");
    currentLink = $("[value=tuition]");
    console.log("current is " + current.text() + ".");
});

$("[value=consultancy]").click(function() {
    $(currentLink).removeClass("cantclick");
    $(current).hide();
    $(this).addClass("cantclick")
    $("#consultancytext").fadeIn(900);
    current = $("#consultancytext");
    currentLink = $("[value=consultancy]");
    console.log("current is " + current.text() + ".");
});

steven35 · Accepted Answer · 2017-11-07 12:40:08Z

1

First create a function that has all your common code:

function handleClick(elem, current, currentLink, text) {
    currentLink.removeClass("cantclick");
    current.hide();
    elem.addClass("cantclick");
    text.fadeIn(900);
    console.log("current is " + current.text() + ".");
}

You can then just use this function and pass in your elements like so:

$("[value=consultancy]").click(function() {
    handleClick($(this), $("#consultancytext"), $("[value=consultancy]"), $("#texts"));
}
// ... add it to the other elements the same way

edited Nov 7, 2017 at 12:40

answered Nov 7, 2017 at 12:29

steven35

4,0676 gold badges40 silver badges50 bronze badges

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

3 Comments

Raph117 Over a year ago

thanks for this, but every time the function is called the currentLink needs to be different because of whichever button was pressed previously... so I can't pre-empt what the currentLink value is...I hope that makes sense

steven35 Over a year ago

How about declaring a global variable and every time a button is clicked assign that element to the variable? So have a global var currentLink; and every time a button is clicked do currentLink = $(this);. You can then use that variable in your functions.

Raph117 Over a year ago

Thanks Steven, that's really helpful. I'll give it a go, R

fdomn-m · Accepted Answer · 2017-11-07 12:28:46Z

The issue is linking the link [value=] to the 'current'.

You can do this via code with multiple ifs, eg:

var linked = "";
var val = $(this).attr("value");
if (val === "home") linked = "texts";
if (val === "aboutme") linked = "aboutmetext";

or the same in a switch, eg:

var linked = "";
switch ($(this).attr("value")) 
{
    case "home": linked="texts"; break;
    case "aboutme": linked="aboutmetext"; break;
}

both of these require maintenance when you add/remove links, so my preferred method is link them via data attributes:

<div class='button-container'>
  <button data-value="home">home</button>
  <button data-value="aboutme">about me</button>
  <button data-value="tuition">tuition</button>
</div>
<div class='texts-container'>
  <div data-link="home">home text</div>
  <div data-link="aboutme">about me text</div>
  <div data-link="tuition">tuition text</div>
</div>

then your code is much simpler:

$("[data-value]").click(function() {
    $(".cantclick").removeClass("cantclick");
    $(this).addClass("cantclick");
    $(".texts-container :visible").fadeOut(900, function() {
        $("[data-link=" + $(this).data("value") + "]").fadeIn(900);
    });

    // can get these dynamically, no need to store them in variables
    // current = $("button.cantclick")
    // currentLink = $("[data-value=" + $("button.cantclick").attr("value") + "]"); 
});

and no need to update it as you add/remove buttons/links.

Might need some work around the fade in/out as :visible may not be correct during animation and will show all the texts to start with (easily fixed) - but the principle is around linking via data- attributes

Collectives™ on Stack Overflow

Refactoring jQuery code into reusable function

2 Answers 2

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related