Помогите упростить код

316
27 декабря 2016, 01:36

Привет всем! Недавно начал познавать мир jQuery и его магию, и состряпал алгоритм для ротатора картинок. Все работает, к моему удивлению, но я бы не проч узнать совета, как сократить мой код, сделать его чище... Просмотрите пожалуйста, есть ли метод упросить мою псанину? Повторюсь, все работает отлично. Вот что у меня есть:

$(document).ready(function () {
   $slide = $('.slide').hide(); 
   $nextBtn = $('#nextBtn');
   $prevBtn = $('#prevBtn');
   $numberOfSlides = $('.slide').length;
   thumbnails = false;
   linkThumbnails = true;
   slideNum = 0;
   slides = [];
   for(i = 1; i <= $numberOfSlides; i++){
      slides.push('#slide'+i);
   };
   primeSlide = slides[slideNum];
   $(primeSlide).show();
   function onClickUp(){
      if (slideNum < slides.length){
         slideNum++;
         var slide = slides[slideNum];
         var prevSlide = slides[slideNum-1];
         $(slide).fadeIn('slow');
         $(prevSlide).fadeOut('slow').delay(200);
         if (slideNum == slides.length){
            slideNum = 0;
            slide = slides[slideNum];
            $(slide).fadeIn('slow');
         }
      }else{ alert('mistake somewhere'); }
   };
   function onClickDown(){
      if (slideNum === 0){
         slideNum = 0;
         $('.slide').eq(slideNum).fadeOut('slow');
         slideNum --;
         $('.slide').eq(slideNum).fadeIn('slow');
         if(slideNum < 0) {
            slideNum = $numberOfSlides -1;
         }
      }else if (slideNum > 0){
         $('.slide').eq(slideNum).fadeOut('slow');
         slideNum --;
         $('.slide').eq(slideNum).fadeIn('slow');
      }   
   };
   $nextBtn.on('click', function () {
      onClickUp();
      $('.thumbnail').removeClass('active'); changeThumb();
      $('.link').removeClass('active'); changeLink();
   });
   $prevBtn.on('click', function () {
      onClickDown();
      $('.thumbnail').removeClass('active');
      changeThumb();
      $('.link').removeClass('active');  changeLink();
   });
   clearTimeout();
   pause = 7000;
   stop = false;
   $('.slider').mouseenter(function(){stop = true; });
   $('.slider').mouseleave(function(){stop = false;});
   function tick() {
      if(!stop){
         onClickUp(); 
         $('.thumbnail').removeClass('active'); 
         changeThumb();
         $('.link').removeClass('active');  
         changeLink();                            
      }
   };
   setInterval(tick, pause); //ЗАПУСКАЕТ АВТОМАТИЧЕСКОЕ ПЕРЕКЛЮЧЕНИЕ
   myArrayImg = [];
   for(i=1; i <= $numberOfSlides; i++){
      myArrayImg.push('slide_'+i+'.png');
   }
   for(i = 0; i < myArrayImg.length; i++ ){
      if(thumbnails){
         $('.thumbnailWrap').append('<div class="thumbnail" id="thumb'+(i+1)+'"><img src="img/'+myArrayImg[i]+'"></div>');
      }else if (!thumbnails & linkThumbnails){
         $('#linkWrap').append('<div class="link" id="link'+(i+1)+'"></div>');  
      }
   }
   myThumbArray = [];
   for(i=1; i <= $numberOfSlides; i++){
      myThumbArray.push('#thumb'+i);
   }
   function changeThumb(){
      var slide = slideNum+1;
      $('.thumbnail').eq(0).addClass('active');
      if(slideNum){
         $.when(slide).then($('#thumb'+slide).addClass('active').focusin());
      }
      if(slideNum > 0){
         $('.thumbnail').eq(0).removeClass('active');
      }
   }
   changeThumb();
   function changeLink(){
      var slide = slideNum+1;
      $('.link').eq(0).addClass('active');
      if(slideNum){
         $.when(slide).then($('#link'+slide).addClass('active'));
      }
      if(slideNum > 0){
         $('.link').eq(0).removeClass('active');
      }
   }
   changeLink();
});

Не судите очень сторого, только только начал в это все вникать. Спасибо!

Answer 1

На исчерпывающий ответ не претендую, но всё же:

0) Захардкожены все DOM-элементы

1) Засоряете глобальную область видимости, забывая про var (две карусели на странице просто сколлапсируют)

2) Каша с обращениям к слайдам, то $('.slide').eq(slideNum), то slide = slides[slideNum];$(slide)

3) Смысл присваивать slide два раза?

slideNum++;
var slide = slides[slideNum];
...
if (slideNum == slides.length){
    slideNum = 0;
    slide = slides[slideNum];

4) Зачем это:

  if (slideNum === 0){
     slideNum = 0;

5) При stop=true просто не меняете слайды, но таймер продолжает работать (согласен, спорный момент. В хороших практиках, правда, рекомендуют использовать вложенный setTimeout вместо setInterval)

6) Это где используется:

myThumbArray = [];
for(i=1; i <= $numberOfSlides; i++){
  myThumbArray.push('#thumb'+i);
}

7) В changeThumb какие-то непонятные манипуляции с первой превью

READ ALSO
Проблема с получением id тега в jquery

Проблема с получением id тега в jquery

В общем, есть страница, где я работаю с ajax и jqueryС одной половины страницы размещены 3 инпута, куда я загоняю инфу, со второй размещена таблица,...

338
Как хранятся картинки в играх? [требует правки]

Как хранятся картинки в играх? [требует правки]

Где в играх хранятся картинки? Я делаю игру на С++, и там надо явно указывать пути к файлам; когда я скачиваю какую-нибудь игру, там нет картинокКуда...

331