Блог

Ниже приведен код, который я написал для переключения между изображениями в моей HTML-карусели. Я определенно знаю, что могу улучшить это. Был бы признателен за любые указатели и все, что я здесь упускаю из виду.

 var i =0;
$(".left-arrow").click(function () {
  i--;
  if (i === -1) {
    i = 2;
    $("#profile-list li:first-child").hide();
    $("#profile-list li:last-child").show();
  } else if (i === 1) {
    $("#profile-list li:last-child").hide();
    $("#profile-list li:nth-child(2)").show();
  } else if (i === 0) {
    $("#profile-list li:nth-child(2)").hide();
    $("#profile-list li:first-child").show();
  }
});

$(".right-arrow").click(function () {
  i  ;
  if (i === 1) {
    $("#profile-list li:first-child").hide();
    $("#profile-list li:nth-child(2)").show();
  } else if (i === 2) {
    $("#profile-list li:nth-child(2)").hide();
    $("#profile-list li:last-child").show();
  } else if (i > 2) {
    i = 0;
    $("#profile-list li:last-child").hide();
    $("#profile-list li:first-child").show();
  }
});```
  

В общем, всегда старайтесь избегать кода, который является почти одинаковым. Также, если вы жестко запрограммировали некоторые значения, такие как 1,2 и -1, это показатель того, что вам нужно оптимизировать эту часть.

Вот пример того, как я бы это сделал:

1. Назовите свои переменные чем-то более понятным: вместо «var i = 0» лучше назовите это что-то вроде

 var currentImageNumber = 0;  

Я согласен, что это может показаться намного длиннее, но поверьте мне, если вы когда-нибудь вернетесь, чтобы прочитать свой код или поделиться им с кем-нибудь еще, вы будете мне благодарны 🙂

2. Вместо жесткого кодирования количества элементов, которые у вас есть, вы можете динамически получать количество:

 var totalImageCount = $('#profile-list li').length;  

3. Что касается логики изменения элемента, она точно такая же для любого количества элементов. Вам нужно только дополнительно позаботиться о 2 крайних случаях (слева от первого элемента и справа от последнего):

 $(".left-arrow").click(function () {
  currentImageNumber--;
  if (currentImageNumber === 0) {
    currentImageNumber = totalImageCount;
  }
  ....  

и

 $(".right-arrow").click(function () {
  currentImageNumber  ;
    if (currentImageNumber > totalImageCount) {
    currentImageNumber = 1;
  }
  
  ...  

4. Теперь все, что осталось сделать, это изменить элемент на currentImageNumber , и поскольку это одинаково как для левой, так и для правой стрелки, лучше всего объединить код в функцию и повторно использовать его в обоих случаях:

 function showImage(imageNumber){
  $("#profile-list li").hide();
    $("#profile-list li:nth-child("   imageNumber   ")").show();
}  

Это полностью рабочий пример со всем кодом в одном месте:

 var totalImageCount = $('#profile-list li').length;
var currentImageNumber = 1;

$(".left-arrow").click(function () {
  currentImageNumber--;
  if (currentImageNumber === 0) {
    currentImageNumber = totalImageCount;
  }

  showImage(currentImageNumber);
});

$(".right-arrow").click(function () {
  currentImageNumber  ;
    if (currentImageNumber > totalImageCount) {
    currentImageNumber = 1;
  }

 showImage(currentImageNumber);
});

function showImage(imageNumber){
  $("#profile-list li").hide();
    $("#profile-list li:nth-child("   imageNumber   ")").show();
}  

 <script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div id="profile-list">
  <ol>
    <li>Item 1</li>
    <li>Item 2</li>
    <li>Item 3</li>
  </ol>
</div>

<button class="left-arrow">Left</button>
<button class="right-arrow">Right</button>  

Вы могли бы сделать все это в одном обработчике событий и проверить, какая из кнопок была нажата, чтобы определить, что делать с i

 var i = 0;

var $items = $('#profile-list li');


$('.left-arrow, .right-arrow').click(function() {
  
  if ($(this).hasClass('left-arrow')) {
    if (--i <= 0) {
      i = $items.length - 1;
    }
  } else {
    if (  i >= $items.length) {
      i = 0
    }
  }
 // hide all and filter the indexed one to show
 $items.hide().eq(i).show()
})  

 #profile-list li {
  display: none
}

#profile-list li:first-child {
  display: block
}  

 <script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<button class="left-arrow arrow-btn">amp;<amp;<</button>
<button class="right-arrow arrow-btn">amp;>amp;></button>
<ul id="profile-list">
  <li>Item 1</li>
  <li>Item 2</li>
  <li>Item 3</li>
  <li>Item 4</li>
</ul>