jQuery - how to simplify the code

Question

<script>
    $(document).ready(function () {

        var lnkAgreement = $('#lnk-agreement');
        if ($('#IsConsultant').attr('checked')) {
            lnkAgreement.attr('href', '@Url.Action("ServiceProviderAgreement", "Home")');
            lnkAgreement.text('service provider agreement');
        }
        else {
            lnkAgreement.attr('href', '@Url.Action("UserAgreement","Home")');
            lnkAgreement.text('user agreement');
        }
        $('#IsConsultant').click(function () {

            if ($('#IsConsultant').attr('checked')) {
                lnkAgreement.attr('href', '@Url.Action("ServiceProviderAgreement", "Home")');
                lnkAgreement.text('service provider agreement');
            }
            else {
                lnkAgreement.attr('href', '@Url.Action("UserAgreement","Home")');
                lnkAgreement.text('user agreement');
            }

        });
    });
</script>
<a href="#" id="lnk-agreement"></a>

How to avoid from iterative blocks of the code?

If you have a recent jQuery version, use `.prop('checked')` instead of `.attr('checked')` — ThiefMaster, Jan 12 '12 at 12:29
You can also chain the jQuery calls together: `lnkAgreement.attr(...).text(...)` — nickf, Jan 12 '12 at 12:30
@ThiefMaster, is there a difference between .prop('') and .attr('')? — Alexandre, Jan 12 '12 at 12:36
The difference between .prop() and .attr() explained: http://stackoverflow.com/questions/5874652/prop-vs-attr — JoJa, Jan 12 '12 at 12:41
@nickf: While true, this clearly doesn't improve the readability in this case. — ThiefMaster, Jan 12 '12 at 12:46

score 1 · Accepted Answer · edited Jan 12 '12 at 12:27

1

Make a function for it:

function Test(){
     if ($('#IsConsultant').attr('checked')) {
          lnkAgreement.attr('href', '@Url.Action("ServiceProviderAgreement", "Home")');
          lnkAgreement.text('service provider agreement');
     }
     else {
          lnkAgreement.attr('href', '@Url.Action("UserAgreement","Home")');
          lnkAgreement.text('user agreement');
    }
 }

edited Jan 12 '12 at 12:27

gdoron

147,333
58
291
367

answered Jan 12 '12 at 12:24

JoJa

612
5
8

score 1 · Answer 2 · answered Jan 12 '12 at 12:28

Put it all in a function.

function myFunction() {
    if ($('#IsConsultant').attr('checked')) {
        lnkAgreement.attr('href', '@Url.Action("ServiceProviderAgreement", "Home")');
        lnkAgreement.text('service provider agreement');
    }
    else {
        lnkAgreement.attr('href', '@Url.Action("UserAgreement","Home")');
        lnkAgreement.text('user agreement');
    }
}

Then call that function on document ready and on the click.

$('#IsConsultant').click(myFunction);

score 1 · Answer 3 · answered Jan 12 '12 at 12:28

function myMethod(){
if ($('#IsConsultant').attr('checked')) {
            lnkAgreement.attr('href', '@Url.Action("ServiceProviderAgreement", "Home")');
            lnkAgreement.text('service provider agreement');
        }
        else {
            lnkAgreement.attr('href', '@Url.Action("UserAgreement","Home")');
            lnkAgreement.text('user agreement');
        }

}

myMethod();

$('#IsConsultant').click(function () {
 myMethod();
}

score 1 · Answer 4 · answered Jan 12 '12 at 12:29

A very quick refactoring gave me this:

<script>
$(document).ready(function () {

    // select the dom elements
    var lnkAgreement = $('#lnk-agreement');
    var isConsultant = $('#IsConsultant');


    setLnkAgreement(isConsultant.attr('checked'));

    isConsultant.click(function () {
        setLnkAgreement(isConsultant.attr('checked')
    });
});

function setLnkAgreement(isConsultant){
    if (isConsultant) {
        lnkAgreement.attr('href', '@Url.Action("ServiceProviderAgreement", "Home")');
        lnkAgreement.text('service provider agreement');
    }
    else {
        lnkAgreement.attr('href', '@Url.Action("UserAgreement","Home")');
        lnkAgreement.text('user agreement');
    }
}

You need to move your common functionality into a function of its own and then clal that with the required parameters.

Regards,

James

score 0 · Answer 5 · answered Nov 09 '12 at 01:02

This?

$(document)
.ready(function () {
var a = $("#lnk-agreement");
$("#IsConsultant")
    .attr("checked") ? (a.attr("href", '@Url.Action("ServiceProviderAgreement", "Home")'), a.text("service provider agreement")) : (a.attr("href", '@Url.Action("UserAgreement","Home")'), a.text("user agreement")), $("#IsConsultant")
    .click(function () {
    $("#IsConsultant")
        .attr("checked") ? (a.attr("href", '@Url.Action("ServiceProviderAgreement", "Home")'), a.text("service provider agreement")) : (a.attr("href", '@Url.Action("UserAgreement","Home")'), a.text("user agreement"))
})
})

David M · Answer 6 · 2012-01-12T12:50:21.720

0

<script>
function chooseAgreementLink() {
    if ($('#IsConsultant').attr('checked')) {
        setAgreementLink("ServiceProviderAgreement", "service provider agreement");
    }
    else {
        setAgreementLink("UserAgreement", "userprovider agreement");
    }
}
function setAgreementLink(controller, text) {
    var lnkAgreement = $('#lnk-agreement');
    lnkAgreement.attr('href', '@Url.Action(controller, "Home")');
    lnkAgreement.text(text);
}
$(document).ready(function () {
    setAgreementLink();
    $('#IsConsultant').click(function () {
        setAgreementLink();
    };
};
</script>

edited Jan 12 '12 at 12:50

answered Jan 12 '12 at 12:27

David M

71,481
13
158
186

You **should** put the `$('#IsConsultant').click()` handler set inside your `document.ready`. – Richard Neil Ilagan Jan 12 '12 at 12:44

score 0 · Answer 7 · answered Jan 12 '12 at 12:40

Something like this should do.

jQuery(function($) {

    var lnkAgreement = $('#lnk-agreement'),
        isConsultant = $('#IsConsultant'),
        handler = function () {
            var isChecked = isConsultant.is(':checked');
            lnkAgreement
                .attr('href', isChecked ? 
                    '@Url.Action("ServiceProviderAgreement", "Home")' : 
                    '@Url.Action("UserAgreement", "Home")'
                    )
                .text(isChecked ? 'service provider agreement' : 'user agreement')
                ;
        };

    isConsultant.on('click', handler);
    handler();

});

Chain your methods when you can,
Encapsulate in a function when you can,
Cache your variables when appropriate.

I'm thinking you can simplify your Razor as well by omitting "Home" from the UrlHelper.Action method if you're calling it from a view within the same controller.

jQuery - how to simplify the code

7 Answers7