Review кода, задача №2

validation.js

'use strict';

(function () {
  var textHashtags = document.querySelector('.text__hashtags');
  var submitButton = document.querySelector('#upload-submit');
  var hashtags = document.querySelector('.text__hashtags');


  var addValidationHashtags = function (hashtag) {
    if (!hashtag) {
      var messageText = '';
    } else if (hashtag[0] !== '#') {
      messageText = textHashtags.setCustomValidity('Хэш-тег должен начинатся с символа # (решётка)');
      hashtags.style.border = '4px solid red';
    } else if (hashtag.length === 1) {
      messageText = textHashtags.setCustomValidity('Хэштег должен содержать значение кроме решетки');
      hashtags.style.border = '4px solid red';
    } else if (hashtag.length > 20) {
      messageText = textHashtags.setCustomValidity('Хэштег может быть длиной не более 20 символов');
      hashtags.style.border = '4px solid red';
    } else if (hashtag.indexOf('#', 1) > 0) {
      messageText = textHashtags.setCustomValidity('Хэштег должен разделяться пробелом');
      hashtags.style.border = '4px solid red';
    } else if (hashtag.match(/[^#a-zA-Z0-9а-яА-Я]/g)) {
      messageText = textHashtags.setCustomValidity('Строка после решётки не может содержать пробелы, спецсимволы (#, @, $ и т.п.), ');
      hashtags.style.border = '4px solid red';
    } else {
      messageText = textHashtags.setCustomValidity('');
    }
    return messageText;
  };


  var formSubmitHandler = function () {
    var hashtagArray = textHashtags.value.toLowerCase().split(' ');
    for (var i = 0; i < hashtagArray.length; i++) {
      var isHashtagValid = addValidationHashtags(hashtagArray[i]);
      if (!isHashtagValid) {
        break;
      }
      if (hashtagArray.indexOf(hashtagArray[i], i + 1) > 0) {
        textHashtags.setCustomValidity('Хэштеги не должны повторяться');
        hashtags.style.border = '4px solid red';
        break;
      }
    }
    if (hashtagArray.length > 5) {
      textHashtags.setCustomValidity('Хэштегов может быть максимум 5');
      hashtags.style.border = '4px solid red';
    }

    if (!textHashtags.validationMessage) {
      textHashtags.setCustomValidity('');
    }
  };


  submitButton.addEventListener('click', formSubmitHandler);


  var clearCustomValidity = function () {
    textHashtags.setCustomValidity('');
  };


  textHashtags.addEventListener('input', clearCustomValidity);
})();

https://gist.github.com/BekoBou/4633b992351bcc5b1bbbd711a9b90c36#file-validation-js-L58

Начнём с самого простого, обработчик стоит на нажатие кнопки, как следствие можно обойти это ограничение, и отправить форму просто нажав на Enter в поле ввода хештегов. Форма не провериться, и уйдёт на сервер. Ну такое. Надо обрабатывать на форме событие submit.

https://gist.github.com/BekoBou/4633b992351bcc5b1bbbd711a9b90c36#file-validation-js-L4-L6

Зачем-то мы ищем дважды один и тот же элемент в DOM и сохраняем — textHashtags и hashtags.

https://gist.github.com/BekoBou/4633b992351bcc5b1bbbd711a9b90c36#file-validation-js-L9-L31 и дальше по коду.

hashtags.style.border = '4px solid red'; — очень много раз повторяется одно и то же, нарушение критерия и так не должно быть. DRY. А если мы завтра поменяем цвет? Будем бегать и менять по коду?

https://gist.github.com/BekoBou/4633b992351bcc5b1bbbd711a9b90c36#file-validation-js-L13 и дальше по коду.

Метод setCustomValidity не возвращает ничего. То есть если результат его работы присвоить, то он будет undefined. Это уже бесполезное действие, а оно дальше за собой влечёт проверку.

https://gist.github.com/BekoBou/4633b992351bcc5b1bbbd711a9b90c36#file-validation-js-L37-L40

Которая всегда положительная. То есть последующие проверки никогда не выполняются, так как он выходит из проверки. Хм…

Нейминг. Например hashtagArray — это же просто hashtags, а как раз hashtags — это скорее всего hashtagsInput.

А если переходить на уровень выше, то в этом модуле вообще не должно быть поиска DOM элементов.

Вариант 1

Так как всё равно другая часть нашего приложения знает про работу с формой, то она знает и о поле с хештегами. Она может передать нам и это поле как параметр к функции валидации. Мы делаем почти такой же код, но при этом не ищем ничего в DOM.

Вариант 2

Примерно то же самое, но на вход функция получает только строку из Input, а на выходе она возвращает либо пустую строку, либо строку с ошибкой. А уже остальная часть, которая отвечает за обработку формы решает какого цвета должны быть ошибки, и что там с ними делать. Такой вариант мне нравится мне больше.

https://gist.github.com/BekoBou/4633b992351bcc5b1bbbd711a9b90c36#file-validation-js-L66

Вообще ненужный обработчик, так как очищать форму надо перед каждой проверкой или на закрытии формы. На input слишком часто срабатывает. Незачем. Да и обработчики остаются в DOM, за ними надо следить и убирать, иначе нарушение критерия.