Need help refactoring this Javascript if/else statement

Question

Here's my current if/else statement:

var current_class = $(this).closest("article").attr("class")

if (current_class == 'opened')
{
  $(this).closest("article").removeClass('opened');
  $(this).closest("article").addClass('closed');
}
else
{
  $(this).closest("article").removeClass('closed');
  $(this).closest("article").addClass('opened');
}

Is there a shorter way to write all of that? Just seems...overweight. :)

mx0 · Accepted Answer · 2010-10-01 18:25:14Z

6

Use

.toggleClass()

Here is an example http://api.jquery.com/toggleClass/

edited Oct 1, 2010 at 18:25

answered Oct 1, 2010 at 18:18

mx0

7,27812 gold badges57 silver badges57 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Ryan Kinal Over a year ago

It seems that .toggleClass() works with two classes. i.e. .toggleClass('opened closed'). Example: jsfiddle.net/xaHTn/4

Dan · Accepted Answer · 2010-10-01 19:56:12Z

2

IMO this is the cleanest and still clearest solution:

var current_class = $(this).closest("article");
if (current_class.hasClass('opened'))
{
  current_class.removeClass('opened').addClass('closed');
}
else
{
  current_class.removeClass('closed').addClass('opened');
}

edited Oct 1, 2010 at 19:56

answered Oct 1, 2010 at 18:20

Dan

3,7662 gold badges27 silver badges24 bronze badges

3 Comments

Dan Over a year ago

the cleanest would be to use only 1 class (open by default?) then only have the class close. Then use toggle like cps7 suggested.

Frank Schwieterman Over a year ago

hasClass() is an important change. Just doing a string comparison of the class attribute will break if the element has other classes added.

not an ai Over a year ago

I would assign a local variable for $(this).closest("article") to avoid doing the DOM lookup twice. That's not a style thing though, it's a speed thing.

user462356 · Accepted Answer · 2010-10-01 18:20:38Z

0

maybe something like:

var current = $(this).closest('article'); // no need to run the selector over and over
var current_class = current.attr('class');
current.removeClass('opened closed').addClass(current_class=='opened'?'closed':'opened');

answered Oct 1, 2010 at 18:20

user462356

Comments

pwb44 · Accepted Answer · 2010-10-01 18:46:23Z

0

$(this).closest("article").toggleClass('opened').toggleClass('closed');

answered Oct 1, 2010 at 18:46

pwb44

1

1 Comment

Dan Over a year ago

You can get a state problem if the item somehow contains both opened and closed.

Collectives™ on Stack Overflow

Need help refactoring this Javascript if/else statement

4 Answers 4

1 Comment

3 Comments

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

3 Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related