5
//Есть массив со всеми картинками на странице
let all_img = document.querySelectorAll('.task-12 img');

//есть кнопка:
document.querySelector('.prev').addEventListener('touchstart', function () {

  all_img.forEach((element, index) => {
    if (element.classList.contains('active-img')) {
      element.classList.remove('active-img');
      all_img[index + 1].classList.add('active-img');
    }
  });

Если я обращаюсь к элементу с индексом + 1 - ничего не работает. Если index - 1 - все ОК. В чем проблема? При чем, если вывести в консоль all_img[index + 1] я получу то, что мне нужно. Но класс к этому элементу я добавить не могу.

ошибка:

script.js:72 Uncaught TypeError: Cannot read property 'classList' of undefined
    at script.js:72
    at NodeList.forEach (<anonymous>)
    at HTMLButtonElement.<anonymous> (script.js:66)
4
  • 2
    Нет. Длина массива равна кол-ву элементов массива. А индексы начинаются с нуля.
    – forzeeh
    9 ноя 2019 в 1:14
  • "если вывести в консоль all_img[index + 1] я получу то, что мне нужно." - нет, не получите.
    – user176262
    9 ноя 2019 в 3:44
  • @Igor Нет, получу. Я получаю в консоли элемент с индексом + 1; Но класс этому элементу я задать не могу.
    – forzeeh
    9 ноя 2019 в 3:47
  • Нет, не получаете. Он там остался от предыдущего касания. Вы можете создать в вопросе работающий пример, демонстрирующий проблему. Используйте кнопку "Фрагмент кода" в редакторе вопроса.
    – user176262
    9 ноя 2019 в 3:51

3 ответа 3

5

если index + 1 изменить на index - 1 все работает.

Не думаю. Ни у одного элемента Вашего массива нет индексированных свойств. Так что до classList дело не доходит.

let a = [1, 2, 3];

a.forEach((element, index) => {
  if (element == 2) {
    console.log(element[index - 1].classList);
  }
});


На последнем элементе Вы залезаете за границу массива.

if (index < all_img.length - 1)
  all_img[index + 1].classList.add('active-img');
2
  • 1
    Спасибо огромное. Вы мне очень помогли.
    – forzeeh
    9 ноя 2019 в 10:52
  • 1
    @forzeeh Пожалуйста. Заслуга - Ваша, так как Вы конструктивно отнеслись к замечаниям по качеству/содержанию вопроса. Желаю успехов.
    – user176262
    9 ноя 2019 в 12:40
4

Допущено три ошибки:

  1. Пренебрежение знанием о методе/типах

//Есть массив со всеми картинками на странице
let all_img = document.querySelectorAll('.task-12 img');

NodeList это не массив. Различие типов важно понимать, чтобы не совершать более грубых ошибок в будущем.

  1. Выход за диапазон коллекции нодов при обращении к ее элементам (как уже ответил Igor)
all_img.forEach((element, index) => {
  // ...
  all_img[index + 1].classList.add('active-img');
  // ...
}); 

 

  1. Алгоритмическая ошибка. Зачем каждому следующему элементу добавлять класс, а на каждой следующей итерации проверять его (класса) наличие, и удалять? Это лишено практической пользы.

Циклы часто являются "узким местом" во многих смыслах, поэтому хорошей привычкой будет всегда сокращать работу в итерациях до разумного минимума (делать только то, что действительно нужно сделать).
К сожалению, очень часто можно видеть как в циклах дублируются операции с DOM, напрасно инстанцируются объекты, множатся одинаковые функции листенеров событий. Сокращение подобной лишней работы - это не преоптимизация. Это повышение качества кода, и четкости его логики.

Решение может быть например таким:

for (var changed, i = all_img.length - 1; i >= 0; i--) {
  all_img[i].classList.remove('active-img'); 
  if (!changed && i && all_img[i - 1].classList.contains('active-img')) {
    all_img[i].classList.add('active-img');
    changed = true; 
  }
}

Пишу с мобилы и без проверки, так что объясню логику (на тот случай если в коде и сам накосячил): перебирая элементы от конца к началу, мы у каждого текущего безусловно удаляем класс, и добавляем обратно только если ранее (за цикл) его не "переставляли вперед", и если предыдущий элемент есть (i > 0), и содержит класс.
Таким образом, одно лишнее действие за весь цикл - это ведь гораздо лучше, чем два лишних действия на каждой его итерации.
И мы яснее видим, в чем итоговые "намерения" этого участка кода, какую микрозадачу он решает.

(предполагаю что читатель ответа автоматически понимает часть выражения !changed && i - при работе в команде с новичками, это можно развернуть на более императивный код с переменными. Еще лучше, когда такие вещи оформлены методами или хелпер-функциями, даже если не планируем их переиспользование)

4
  • Вопрос о forEach, но этот метод неспособен перебирать в обратном порядке (чего требует задача, насколько я ее понял) - поэтому предлагаемое решение использует for.
    – yar85
    9 ноя 2019 в 11:56
  • Алгоритмическая ошибка...Это лишено практического смысла. - как по мне, лишено смысла каждый раз проходить весь объект эл-тов чтобы передвинуть "указатель"; и тут весь цикл это сплошное лишнее движение.
    – InDevX
    9 ноя 2019 в 12:04
  • @InDevX, цикл является лишним усложнением только если классы не могут меняться другим кодом. К тому же, проблема из вопроса тесно связана с логикой циклов, и ответы без ее объяснения фактически являются оффтопом. Цель моего ответа прежде всего в том чтобы указать на общие повторяемые ошибки (особенно #3), а не в том чтобы решить единично-индивидуальную задачу ТСа.
    – yar85
    9 ноя 2019 в 12:09
  • Чтобы было понятнее, дополнил ответ раскрытием мысли о важности алгоритма применительно к циклам.
    – yar85
    9 ноя 2019 в 14:12
1

// т.е. forEach не видит элементы, по которым он еще не прошелся?

Он проходится по всем эл-там и всё видит.

У вас проблема в том что вы пытались к другому эл-ту массиву "достучаться" через текущий эл-т.

let a = [1,2,3];

a.forEach((element, index) => {
 if(element == 2) {
  console.log(a[index + 1]);
 }
});

Зачем вам все картинки в массиве и циклом по ним проходиться? Можно как-то так это делать:

document.querySelector('.prev').addEventListener('click', function (){
    let activeImg = document.querySelector('.task-12 img.active-img'),
        nextImg = activeImg.nextElementSibling,
        allImg = document.querySelectorAll('.task-12 img');
        
    if (allImg.length > getChildNumber(activeImg)+1) {
        activeImg.classList.remove('active-img');
        nextImg.classList.add('active-img'); 
    } else {
        activeImg.classList.remove('active-img');
        allImg[0].classList.add('active-img');
    }
    
});

function getChildNumber(element) {
  return Array.from(element.parentNode.children).indexOf(element);
}
.task-12 img {
  width: 25px;
  height: 25px;
  padding: 5px;
}
.active-img {
  border: 3px solid #000;
}
<div class="task-12">
  <img class="active-img"  src="https://hsto.org/getpro/habr/post_images/c1a/e7e/e3d/c1ae7ee3d053f0809e9d22c320924a18.jpg" alt="">
  <img src="https://hsto.org/getpro/habr/post_images/c1a/e7e/e3d/c1ae7ee3d053f0809e9d22c320924a18.jpg" alt="">
  <img  src="https://hsto.org/getpro/habr/post_images/c1a/e7e/e3d/c1ae7ee3d053f0809e9d22c320924a18.jpg" alt="">
  <img src="https://hsto.org/getpro/habr/post_images/c1a/e7e/e3d/c1ae7ee3d053f0809e9d22c320924a18.jpg" alt="">
  <img src="https://hsto.org/getpro/habr/post_images/c1a/e7e/e3d/c1ae7ee3d053f0809e9d22c320924a18.jpg" alt="">
</div>
<button class="prev">next</button>

11
  • 1
    "априори" - ? (2 символа нужно...)
    – user176262
    9 ноя 2019 в 2:38
  • Так я о текущем forEach, а не о возможностях его
    – InDevX
    9 ноя 2019 в 2:39
  • @InDevX "I don't think it means what you think it means."
    – user176262
    9 ноя 2019 в 3:03
  • @forzeeh в вопрос, все в вопрос
    – user176262
    9 ноя 2019 в 3:04
  • @Igor возможно, тут я вас, не совсем понял или совсем не понял, пока что не понял насколько я не понял
    – InDevX
    9 ноя 2019 в 3:27

Ваш ответ

By clicking “Отправить ответ”, you agree to our terms of service and acknowledge you have read our privacy policy.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.