Разбор задачки про события

text.html

Была задача.
Сделать квадрат, который по клику меняет цвет (серый -> синий -> желтый -> зеленый).

Все по идее просто и понятно, а присылают вам такой (работающий!) код:
<style>
  .sq {
    width: 100px;
    height: 100px;
    margin: 15px;
    background-color: gray;
  }
  .blue {
    background-color: blue;
  }
  .yellow {
    background-color: yellow;      
  }
  .green {
    background-color: green;      
  }
</style>

<div class="sq"></div>

<script>
  var $sq = document.querySelector('.sq');
  
  $sq.addEventListener('click', function() { // навешиваем первый обработчик события (1)
    this.classList.toggle('blue');
    this.addEventListener('click', function() { // навешиваем первый обработчик события (2)
      this.classList.toggle('yellow');
      this.addEventListener('click', function() { // навешиваем первый обработчик события (3)
        this.classList.toggle('green');
      });
    });        
  });
</script>

Давайте разбираться.

Сначала всё понятно: получаем DOM-элемент, навешиваем обработчик события.

Что происходит при первом клике?
сработал 1 (добавил синий)
2 навесили

Что происходит при клике на том же квадратике?
сработал 1 (удалил синий)
2 навесили
Все точно так же как и в первый раз, но в первый раз был навешен 
новый обработчик, и теперь он тоже сработает:
	сработал 2 (добавил желтый)
	3 навесили

3-й клик:
сработал 1 (добавил синий)
2 навесили
сработал 2 (удалил желтый)
3 навесили
	сработал 2 (добавил желтый)
	3 навесили
	сработал 3 (добавил зеленый)
Сейчас див с тремя классами, но он будет зеленого цвета, 
так как по счастливой случайности в цсс этот класс написан последним. 

4 клик
сработал 1 (удалил синий) 
2 навесили
сработал 2 (удалил желтый)
3 навесили
сработал 2 (добавил желтый)
3 навесили 
сработал 3 (удалил зеленый)
сработал 2 (удалил желтый)
3 навесили
сработал 3 (добавил зеленый)
сработал 3 (удалил зеленый)
Теперь див без дополнительных классов, только sq, и он снова серый.

5 клик
сработал 1 (добавил синий) 
2 навесили
сработал 2 (добавил желтый)
3 навесили
сработал 2 (удалил желтый)
3 навесили 
сработал 3 (добавил зеленый)
сработал 2 (добавил желтый)
3 навесили
сработал 3 (удалил зеленый)
сработал 3 (добавил зеленый)
сработал 2 (удалил желтый)
3 навесили
сработал 3 (удалил зеленый)
сработал 3 (добавил зеленый)
сработал 3 (удалил зеленый)

Ужас кароч.

При каждом следующем клике будет еще 1 раз навешиваться второй обработчик, 
а третий обработчик с четвертого клика будет навешиваться столько раз,
сколько он уже выполнился + 1. То есть при каждом клике количество
срабатываемых событий на одном элементе растёт в числовой 
последовательности (1, 3, 6, 10, 15...). Так же растёт и количество 
обработчиков событий, которые навешиваются при одном клике. 
Можно представить, насколько это замедляет выполнение кода. И, кстати, 
в этом примере это видно наглядно: через какое-то количество кликов заметно,
что смена цвета происходит медленнее. А если поставить в каждом обработчике
по консоль логу, то можно весь этот ад увидеть в консоле.

Правильнее сделать так, чтобы при клике происходила проверка класса,
который уже есть, и в зависимости от этого добавлять другой.
Например, так:

<script>
  var $sq = document.querySelector('.sq');

  $sq.addEventListener('click', function() {
    if (this.classList.contains('yellow')) {
      this.classList.remove('yellow');
      this.classList.add('blue');
      return;
    }

    if (this.classList.contains('green')) {
      this.classList.remove('green');
      this.classList.add('yellow');
      return;
    }

    if (this.classList.contains('blue')) {
      this.classList.remove('blue');
      this.classList.add('green');
      return;
    }

    if (!this.classList.contains('blue')) {
      this.classList.add('blue');
    }
  });
</script>
	
Ну или если хочется покороче, то можно позаигрывать со свойством className:

<script>
  var $sq = document.querySelector('.sq');
  var colors = ['', 'blue', 'yellow', 'green'];
  var count = 1;

  $sq.addEventListener('click', function() {
    this.className = 'sq ' + colors[count % 4];
    count++;
  });
</script>

Если нужна зависимость следующего цвета от текущего, то можно сделать хэш типа:

const colors = {
  blue: 'yellow',
  yellow: 'green',
  green: 'blue',
}

С моей точки зрения, это не лучший способ ответить на ревью кода джуна, которого менторишь.
Да, тут все расписано очень детально, и написание таких подробных и понятных текстов развивает ментора, но:

• Не учит думать джуна (даёт рыбу вместо удочки)
• Вызывает скорее негативные эмоции ("блин, ну я и наговнякал, неуч" вместо "смотри, я нашел правильное решение"). Эмоции важны

Я считаю что хороший ментор должен не дать ответ, а помочь задать правильный вопрос. Чтобы джун смог понять что не так, посоображать, подумать, найти нужную спеку и отыскать решение сам. Ведь он инжинер, именно в этом заключается его будущая работа, именно этому ментор должен его научить.
А после того как джун нашел ответ и доволен можно уже и попытаться обобщить знания и рассказать ему более top level взгляд

Как бы поступил я.
Я бы дал пару ченж реквестов на потестить код, в зависимости от контекста:

• Что будет если поменять порядок классов в цсс?
• Добавь консоль лог. Почему так много?
• Покликай 100 раз. Отчего тормозит?

После демонстрации хрупкости решения к правкам нужно дать направление копки. (Иначе можно и Maximum call stack size exceeded джуну сделать после 500 ревью.) Тут сложнее, наверное я бы попросил его нарисовать блок-схему кода, или high level псевдокод, или просто описать максимально лаконично и машинно условие задачи; потом в диалоге попытался сделать его ответ максимально лаконичным, и потом попросил превратить это в код не нарушая созданной лаконичной структуры.
После успешного ответа и, возможно, мелких фиксов я бы обобщил ответ до тезисов: код должен самым тупым и понятным и буквальным образом делать то что поставлено в задаче. Код с ухищрениями сложно читать и он часто хрупкок к малейшим изменениям.

Ты можешь подвести ученика к двери, но войти должен он сам. Если его в нее внести - то вряд ли он совладает со следующей