CODEWITHSUNDEEP
javascriptjquery
styler
styler

Reputation: 16501

How can I improve this toggleClass function containing basically repeated logic?

I have a toggleClass function that basically reverses the classes in an if..else depending on whether the state is a delete or reset. Can anyone offer any suggestions on how I can improve this to make it better?

JS

/**
 * Uses the click events currentTarget as the element and a state string
 * to work out how the required classes should be toggled.
 * 
 * @param $el - jQuery click event
 * @param state - string `reset` or `delete`
 */
toggleClass: function($el, state) {

    console.log('toggleClass', state);

    if(state === 'delete') {
        $el.toggleClass('js-delete-session js-reset-session');
        $el.toggleClass('fa-times fa-undo');
    } else {
        $el.toggleClass('js-reset-session js-delete-session');
        $el.toggleClass('fa-undo fa-times');
    }

},

Upvotes: 2

Views: 72

Answers (2)

Tushar
Tushar

Reputation: 87213

The first statement to toggle class js-delete-session and js-reset-session is common in both if and else block, it can be moved outside of if else.

toggleClass: function($el, state) {
    // Common to both delete and reset
    $el.toggleClass('js-delete-session js-reset-session');

    if(state === 'delete') {
        $el.toggleClass('fa-times fa-undo');
    } else {
        $el.toggleClass('fa-undo fa-times');
    }
},

Also, ternary operator can be used as option to if...else

toggleClass: function($el, state) {
    $el.toggleClass('js-delete-session js-reset-session');
    state === 'delete' ? $el.toggleClass('fa-times fa-undo') : $el.toggleClass('fa-undo fa-times');
},

OR,

toggleClass: function($el) {
    $el.toggleClass('js-delete-session js-reset-session fa-times fa-undo');
},

Upvotes: 2

Guffa
Guffa

Reputation: 700630

As you are toggling the same classes in both states, you don't need separate code for the states. You can even put the classes in the same call:

toggleClass: function($el) {
  $el.toggleClass('js-delete-session js-reset-session fa-times fa-undo');
},

Upvotes: 4

Related Questions