2

I encounter this issue a lot when I'm trying to create a website. As I'm probably not the only one that encounters this issue I'd thought to share my issue:

switch (input) {
  case "home":
    $('#slide1').removeClass('hideSlide');
    $('#slide2').addClass('hideSlide');
    $('#slide3').addClass('hideSlide');
    $('#slide4').addClass('hideSlide');
    $('#slide5').addClass('hideSlide');
    break;
  case "bio":
    $('#slide1').addClass('hideSlide');
    $('#slide2').removeClass('hideSlide');
    $('#slide3').addClass('hideSlide');
    $('#slide4').addClass('hideSlide');
    $('#slide5').addClass('hideSlide');
    break;
  case "ref":
    $('#slide1').addClass('hideSlide');
    $('#slide2').addClass('hideSlide');
    $('#slide3').removeClass('hideSlide');
    $('#slide4').addClass('hideSlide');
    $('#slide5').addClass('hideSlide');
    break;
  case "dit":
    $('#slide1').addClass('hideSlide');
    $('#slide2').addClass('hideSlide');
    $('#slide3').addClass('hideSlide');
    $('#slide4').removeClass('hideSlide');
    $('#slide5').addClass('hideSlide');
    break;
  case "cont":
    $('#slide1').addClass('hideSlide');
    $('#slide2').addClass('hideSlide');
    $('#slide3').addClass('hideSlide');
    $('#slide4').addClass('hideSlide');
    $('#slide5').removeClass('hideSlide');
    break;
  case "closeMenu":
    closeMenu();
    break
  default:

    break;
}

So my question was: How can I simplify or shorten this piece of code?

It would be very nice if some one could help me out by showing me an example or by referring me to a solution.

Thanks in advance!

yuriy636
  • 11,171
  • 5
  • 37
  • 42

6 Answers6

5

One way to do this is as follows:

$('#slide1, #slide2, #slide3, #slide4, #slide5').addClass('hideSlide');

switch (input) {
  case "home":
    $('#slide1').removeClass('hideSlide');
    break;
  case "bio":
    $('#slide2').removeClass('hideSlide');
    break;
  case "ref":
    $('#slide3').removeClass('hideSlide');
    break;
  case "dit":
    $('#slide4').removeClass('hideSlide');
    break;
  case "cont":
    $('#slide5').removeClass('hideSlide');
    break;
  case "closeMenu":
    closeMenu();
    break
  default:
    break;
}

This leverages jQuery's multiple selector and adds the class hideSlide to all the slides in one statement, and then removes the class from the required slide in the switch case.

31piy
  • 23,323
  • 6
  • 47
  • 67
2

Add a class to all links / slides - on the click event use the class name to add the hideSlide class and then the id of the clicked element to remove it, The following requires the "slides" class to be added to the links - then based on the click of any of them - applies the hideSlide class to all them and then removes it from the clicked element using the ID of that link.

$('.slides').click(function() {
  $('.slides').addClass('hideSlide');
  let id = $(this).attr('id');
  ('#' +id).removeClass('hideSlide');
})
gavgrif
  • 15,194
  • 2
  • 25
  • 27
1

add all Classes before switch case. In switch case you only remove the class

1

Try something like this

function showSlide(id){
    $('.slides').addClass('.hideSlide');
    $('#'+id).removeClass('.hideSlide');
}

showSlide('your-slide-id');

All slides should be given the class slides and each one of them should have it's own id that you can use to add the class.

IsuNas Labs
  • 136
  • 1
  • 5
1

The most obvious refactoring is to do this

$(someSelectorTargetingAllSlides).addClass('hideSlide');

switch (input) {
  case "home":
    $('#slide1').removeClass('hideSlide');
    break;
  case "bio":
    $('#slide2').removeClass('hideSlide');
    break;
  case "ref":
    $('#slide3').removeClass('hideSlide');
    break;
  case "dit":
    $('#slide4').removeClass('hideSlide');
    break;
  case "cont":
    $('#slide5').removeClass('hideSlide');
    break;
  case "closeMenu":
    closeMenu();
    break
  default:

    break;
}

Then as a next step to get rid of the ugly and verbose switch statement, you could map the input values to the selectors:

$(someSelectorTargetingAllSlides).addClass('hideSlide');

var actionMap = {
 home: '#slide1',
 bio: '#slide2',
 ref: '#slide3'
 dit: '#slide4',
 cont: '#slide5'
}

if (actionMap[input]) {
  $(actionMap[input]).removeClass('hideSlide');
}
else if (input == "closeMenu") {
  closeMenu();
}

That's still far from perfect code, but it's much more succinct and DRY.

DanSingerman
  • 36,066
  • 13
  • 81
  • 92
1
function doSomething(elementToHide)
{
    var array = [
    '#slide1',
    '#slide2',
    '#slide3',
    '#slide4',
    '#slide5',
  ]

  array.foreach(function (item) {
        if (item === elementToHide)
      {
        $(item).removeClass('hideSlide')
      }
      else {
        $(item).addClass('hideSlide')
      }
  })
}

This way you only need to call doSomething('#slideX') once every case.

P_Andre
  • 730
  • 6
  • 17