CODEWITHSUNDEEP
javascript
lin
lin

Reputation: 221

How can I improve repeating If statements?

I try to write some scroll effect and its success, but I think it would be done better and I need to some advice to simplify these code.

var nowIndex = 0;
var pcMenuSelect = $('body').find('.carFormatMenu li');
var mbMenuSelect = $('body').find('.menu-cartype .item');
var dropText = mbMenuSelect.parents('.dropdown').find('.text');
var dropValue = mbMenuSelect.parents('.dropdown').find('input')

if (nowIndex == 0){
    dropText.text(mbMenuSelect.eq(0).text());
    pcMenuSelect.removeClass('current');
    pcMenuSelect.eq(0).addClass('current');

    mbMenuSelect.removeClass('active selected');
    mbMenuSelect.eq(0).addClass('active selected');

    dropValue.eq(0).val(mbMenuSelect.eq(0).text());
}
if (nowIndex == 1){
    dropText.text(mbMenuSelect.eq(1).text());
    pcMenuSelect.removeClass('current');
    pcMenuSelect.eq(1).addClass('current');
    mbMenuSelect.removeClass('active selected');
    mbMenuSelect.eq(1).addClass('active selected');
    dropValue.eq(1).val(mbMenuSelect.eq(1).text());
}
if (nowIndex == 2){
    dropText.text(mbMenuSelect.eq(2).text());
    pcMenuSelect.removeClass('current');
    pcMenuSelect.eq(2).addClass('current');
    mbMenuSelect.removeClass('active selected');
    mbMenuSelect.eq(2).addClass('active selected');
    dropValue.eq(2).val(mbMenuSelect.eq(2).text());
}
if (nowIndex == 3){
    dropText.text(mbMenuSelect.eq(3).text());
    pcMenuSelect.removeClass('current');
    pcMenuSelect.eq(3).addClass('current');
    mbMenuSelect.removeClass('active selected');
    mbMenuSelect.eq(3).addClass('active selected');
    dropValue.eq(3).val(mbMenuSelect.eq(3).text());

}
if (nowIndex == 4){
    dropText.text(mbMenuSelect.eq(4).text());
    pcMenuSelect.removeClass('current');
    pcMenuSelect.eq(4).addClass('current');

    mbMenuSelect.removeClass('active selected');
    mbMenuSelect.eq(4).addClass('active selected');
    dropValue.eq(4).val(mbMenuSelect.eq(4).text());

}
if (nowIndex == 5){
    dropText.text(mbMenuSelect.eq(5).text());
    pcMenuSelect.removeClass('current');
    pcMenuSelect.eq(5).addClass('current');
    mbMenuSelect.removeClass('active selected');
    mbMenuSelect.eq(5).addClass('active selected');
    dropValue.eq(5).val(mbMenuSelect.eq(5).text());

}
if (nowIndex == 6){
    dropText.text(mbMenuSelect.eq(6).text());
    pcMenuSelect.removeClass('current');
    pcMenuSelect.eq(6).addClass('current');
    mbMenuSelect.removeClass('active selected');
    mbMenuSelect.eq(6).addClass('active selected');
    dropValue.eq(6).val(mbMenuSelect.eq(6).text());

}
if (nowIndex == 7){
    dropText.text(mbMenuSelect.eq(7).text());
    pcMenuSelect.removeClass('current');
    pcMenuSelect.eq(7).addClass('current');
    mbMenuSelect.removeClass('active selected');
    mbMenuSelect.eq(7).addClass('active selected');
    dropValue.eq(7).val(mbMenuSelect.eq(7).text());

}

Upvotes: 0

Views: 59

Answers (4)

Tanul
Tanul

Reputation: 354

Addition to Dinesh's answer:

var nowIndex = 0;
    var pcMenuSelect = $('body').find('.carFormatMenu li');
    var mbMenuSelect = $('body').find('.menu-cartype .item');
    var dropMenu=mbMenuSelect.parents('.dropdown'); 

    if (nowIndex> 0){
        dropMenu.find('.text').text(mbMenuSelect.eq(nowIndex).text());
        pcMenuSelect.removeClass('current').eq(nowIndex).addClass('current');
        mbMenuSelect.removeClass('active selected').eq(nowIndex).addClass('active selected');
        dropMenu.find('input').eq(nowIndex).val(mbMenuSelect.eq(nowIndex).text());
    }

I've done caching of mbMenuSelect. But not of $('body') because running find function again on whole body will slow the execution.

Upvotes: 1

synthet1c
synthet1c

Reputation: 6282

make a function that encapsulates the common functionality.

var pcMenuSelect = $('body').find('.carFormatMenu li')
var mbMenuSelect = $('body').find('.menu-cartype .item')
var dropText = mbMenuSelect.parents('.dropdown').find('.text')
var dropValue = mbMenuSelect.parents('.dropdown').find('input')

function updateSelect(index) {
  dropText.text(mbMenuSelect.eq(index).text())
  pcMenuSelect
    .removeClass('current')
    .eq(index)
    .addClass('current')

  mbMenuSelect
    .removeClass('active selected')
    .eq(index)
    .addClass('active selected')

  dropValue.eq(index).val(mbMenuSelect.eq(index).text())
}
// call the function with the index
updateSelect(nowIndex)

Upvotes: 0

Dinesh undefined
Dinesh undefined

Reputation: 5546

addition to LalithKumar's answer use chaining also. 

var nowIndex = 0;
    var pcMenuSelect = $('body').find('.carFormatMenu li');
    var mbMenuSelect = $('body').find('.menu-cartype .item');
    var dropText = mbMenuSelect.parents('.dropdown').find('.text');
    var dropValue = mbMenuSelect.parents('.dropdown').find('input');

if (nowIndex>0){
    dropText.text(mbMenuSelect.eq(nowIndex).text());
    pcMenuSelect.removeClass('current').eq(nowIndex).addClass('current');

    mbMenuSelect.removeClass('active selected').eq(nowIndex).addClass('active selected');

    dropValue.eq(nowIndex).val(mbMenuSelect.eq(nowIndex).text());
}

Upvotes: 1

lalithkumar
lalithkumar

Reputation: 3530

Just do like this:

var nowIndex = 0;
    var pcMenuSelect = $('body').find('.carFormatMenu li');
    var mbMenuSelect = $('body').find('.menu-cartype .item');
    var dropText = mbMenuSelect.parents('.dropdown').find('.text');
    var dropValue = mbMenuSelect.parents('.dropdown').find('input');

if (nowIndex>0){
    dropText.text(mbMenuSelect.eq(nowIndex).text());
    pcMenuSelect.removeClass('current');
    pcMenuSelect.eq(nowIndex).addClass('current');

    mbMenuSelect.removeClass('active selected');
    mbMenuSelect.eq(nowIndex).addClass('active selected');

    dropValue.eq(nowIndex).val(mbMenuSelect.eq(nowIndex).text());
}

Upvotes: 1

Related Questions