Switching some text labels when the first image of a carousel is active

Question

Is there any way I can simplify this inefficient piece of JavaScript?

$(function () {
    var $carousel = $('#carousel');
    var $switch = $('#switch');
    var $header = $('#header');
    var $submit = $('#submit');

    $carousel.bind('slid', function() {
        var index = $('#carousel .item').index($('#carousel .carousel-inner .active'));
        if (index == 0) {
            $header.text('Sign In');
            $switch.text('Sign Up');
            $submit.text('Sign In');
            $submit.attr('form', 'sign_in');
        } else {
            $header.text('Sign Up');
            $switch.text('Sign In');
            $submit.text('Sign Up');
            $submit.attr('form', 'sign_up');
        }
    });
});

Answer 1

You could do this:

$(function () {
    var $carousel = $('#carousel');
    var $switch = $('#switch');
    var $headerSubmit = $('#header, #submit');
    var $submit = $('#submit');
    var text_strings = [ 'Sign In', 'Sign Up' ];
    var form_strings = [ 'sign_in', 'sign_up' ];

    $carousel.bind('slid', function() {
        var index = $('#carousel .item').index($('#carousel .carousel-inner .active'));
        var sign_in = (index == 0);
        $headerSubmit.text(text_strings[sign_in ? 1 : 0]);
        $switch.text(text_strings[sign_in ? 0 : 1]);
        $submit.attr('form', form_strings[sign_in ? 1 : 0]);
    });
});

If you want to reduce the lines of code (with a slight negative impact to performance if this function gets called a lot), you could do this:

$(function () {
    var text_strings = [ 'Sign In', 'Sign Up' ];
    var form_strings = [ 'sign_in', 'sign_up' ];

    $('#carousel').bind('slid', function() {
        var index = $('#carousel .item').index($('#carousel .carousel-inner .active'));
        var sign_in = (index == 0);
        $('#header, #submit').text(text_strings[sign_in ? 1 : 0]);
        $('#switch').text(text_strings[sign_in ? 0 : 1]);
        $('#submit').attr('form', form_strings[sign_in ? 1 : 0]);
    });
});

Answer 2

It seems like you're using Bootstrap's Carousel? I've moved the two texts of the buttons to variables so that if you're using a minification tool (UglifyJs, Google's Closure Compiler etc), it'll help with minification.

I find using the index really reduces the readability of the code, so I've declared a variable isSignUp. This way, you reduce the cognitive load of the user reading the code. You don't have to remember which index is which after this declaration.

Since the .items are already on the page, you can cache their lookup with $carouselItems and then find the .active one with .index(). A better approach could be to check for a specific class rather than index.

Shorter code isn't always the best approach. I find that the current answer is difficult to read.

$(function () {
    var $carousel = $('#carousel');
    var $carouselItems = $carousel.find('.carousel-inner .item');
    var $switch = $('#switch');
    var $header = $('#header');
    var $submit = $('#submit');

    var SIGN_IN = 'Sign In';
    var SIGN_UP = 'Sign Up';

    $carousel.on('slid', function() {
      var isSignIn = $carouselItems.index('.active') === 0;

      if (isSignIn) {
        $header.text(SIGN_IN);
        $switch.text(SIGN_UP);
        $submit.text(SIGN_IN).attr('form', 'sign_in');
      }
      else {
        $header.text(SIGN_UP);
        $switch.text(SIGN_IN);
        $submit.text(SIGN_UP).attr('form', 'sign_up');
      }
    });
});