javascript - How can I refactor this jQuery code? - Stack Overflow

admin管理员组

文章数量:1312782

The code below is for a simple newsletter signup widget.

I'm sure there's a way to make it more concise, any ideas?

var email_form = $('.widget_subscribe form'); 
var email_submit = $('.widget_subscribe .submit');
var email_link = $('.widget_subscribe .email');

// Hide the email entry form when the page loads
email_form.hide();

// Show the form when the email link is clicked
$(email_link).click( function () {
    $(this).toggle();
    $(email_form).toggle();
    return false;
});

// Hide the form when the form submit is clicked
$(email_submit).click( function () {
    $(email_link).toggle();
    $(email_form).toggle();
});

// Clear/reset the email input on focus 
$('input[name="email"]').focus( function () {
    $(this).val("");
    }).blur( function () {
        if ($(this).val() == "") {
            $(this).val($(this)[0].defaultValue);
        }
});

The code below is for a simple newsletter signup widget.

I'm sure there's a way to make it more concise, any ideas?

var email_form = $('.widget_subscribe form'); 
var email_submit = $('.widget_subscribe .submit');
var email_link = $('.widget_subscribe .email');

// Hide the email entry form when the page loads
email_form.hide();

// Show the form when the email link is clicked
$(email_link).click( function () {
    $(this).toggle();
    $(email_form).toggle();
    return false;
});

// Hide the form when the form submit is clicked
$(email_submit).click( function () {
    $(email_link).toggle();
    $(email_form).toggle();
});

// Clear/reset the email input on focus 
$('input[name="email"]').focus( function () {
    $(this).val("");
    }).blur( function () {
        if ($(this).val() == "") {
            $(this).val($(this)[0].defaultValue);
        }
});

• javascript
• jquery

Share Improve this question Follow asked May 5, 2009 at 13:44 meleyalmeleyal 33.3k2424 gold badges7575 silver badges7979 bronze badges 4

• Looks pretty good. You only miss ); after the last blur function. – Pim Jager Commented May 5, 2009 at 13:48
• Looks quite ok to me. You don't have to $() the variables like $(email_form) because that's been done when instatiating them. – Rashack Commented May 5, 2009 at 13:50
• As a sidenote... when I make a typo in my e-mail address and attempt to correct it your code will pletely eradicate what I had typed before, forcing me to start over again. (This happens a LOT on websites and it sucks.) – Huppie Commented Jul 1, 2009 at 13:00
• you can drop the second and third var statement and just use ',' at the end. – Mark Rogers Commented Dec 3, 2009 at 16:27

Add a ment  | 

4 Answers 4

Sorted by: Reset to default 11

You have some similar code here.

// Show the form when the email link is clicked
$(email_link).click( function () {
        $(this).toggle();
        $(email_form).toggle();
        return false;
});

// Hide the form when the form submit is clicked
$(email_submit).click( function () {
        $(email_link).toggle();
        $(email_form).toggle();
});

It could be refactored so the similarity is obvious.

// Show the form when the email link is clicked
$(email_link).click( function () {
        $(email_link).toggle();
        $(email_form).toggle();
        return false;
});

// Hide the form when the form submit is clicked
$(email_submit).click( function () {
        $(email_link).toggle();
        $(email_form).toggle();
});

So you could wrap toggling the link and the form into a function.

var toggleEmailLinkAndForm = function () {
        $(email_link).toggle();
        $(email_form).toggle();
}   

$(email_link).click(toggleEmailLinkAndForm);
$(email_submit).click(toggleEmailLinkAndForm);

And as others have pointed out, you can drop the redunant $()s.

var toggleEmailLinkAndForm = function () {
        email_link.toggle();
        email_form.toggle();
}   

email_link.click(toggleEmailLinkAndForm);
email_submit.click(toggleEmailLinkAndForm);

It's already pretty concise, there's not much more you can do.

Anywhere you have $(email_submit) you can just have email_submit, because you've already wrapped it in $() (which makes it a jquery object).

Eg:

email_submit.click( function () {
    email_link.toggle();
    email_form.toggle();
});

I like Patrick McElhaney Code Best.

toggleEmailLinkAndForm() {
    email_link.toggle();
    email_form.toggle();
}   

email_link.click(toggleEmailLinkAndForm);
email_submit.click(toggleEmailLinkAndForm);

Part of refactoring is not going overboard. I would not remend creating an extra click event that calls the other click event. The point of refactoring is readability and flexibility. You can also use the jQuery method "add" to shrink the code but it will bee even harder to read.

email_link.add(email_submit).click(function(){
    email_link.add(email_form).toggle();
});

Like Patrick said (+1), and you can also skip the extra function:

email_submit.click(function () {
    email_link.toggle();
    email_form.toggle();
});
email_link.click(function () {
    email_submit.click(); //calls the click function already subscribed
    return false;
});

本文标签： javascriptHow can I refactor this jQuery codeStack Overflow