Задать вопрос

Todolist на javascript, что исправить?

Здравствуйте. Создал такой todolist на чистом javascript - https://jsfiddle.net/yf581z6b/ . Над версткой я сильно не заморачивался. Хотел бы узнать ваше мнение по поводу того как я реализовал все это на js, где есть ошибки или может есть получше способ как это все сделать.
  • Вопрос задан
  • 3474 просмотра
Подписаться 5 Оценить Комментировать
Решения вопроса 1
@kahi4
может есть получше способ как это все сделать.


Очевидно, что способ по-лучше это сделать -- использовать какую-то библиотеку, покуда с ростом кода вы начнете рефакторить и рано или поздно все равно получите свой велосипед, только колеса у него могут оказаться квадратненькими. Но на Vanila, так на vanila.

Первое: безусловно нужно подсмотреть сюда todomvc за вдохновением.

Второе:
document.querySelector('.add-todo').addEventListener(...)

Плохая идея. Вообще плохой код по нескольким причинам.
- сам listener лучше вынести отдельно, проще отслеживать и рефакторить, а главное -- вот так созданную анонимную функцию потом отписать не получится от события, что порождает мало того что утечку памяти, так еще и неприятные эффекты, если вдруг все же понадобится это делать.
- что, если элементов с .add-todo будет два?
- вообще хороший тон (если не прибегать к библиотекам) является либо использование id (в данном случае это подходит, покуда кнопка может быть только одна, но тоже лучше не злоупотреблять, покуда когда-то может появиться вторая), либо класс js-add-todo (можно использовать другой префикс, главное -- однозначное понимание какой класс отвечает за стилизацию, а какой за логику. Практика показывает, что в процессе мелких правок по дизайну верстка может поменяться значительно и есть шанс запутаться в классах), либо ныне модно использовать data-аттрибут

Дальше.
function(e){
	var field = document.querySelector('.field-todo');

	if (field.value && field.value !== ' ') {
		addTodo(field);
		field.value = '';
	}
}


Тут прям много чего плохо. Почему field ищется в доме каждый раз, а не сохраняется в скоупе приложения? (о нем еще поговорим). Проверка на равенство одному пробелу перестанет работать как минимум с двумя пробелами и так дальше. Так же было бы неплохо вообще создать метод isFieldClear, в котором инкапсулировать проверку. Почему в addTodo передается field, а не field.value? А что если input сменится на, не знаю там, div с contentEditable?
Да и вообще, извлечение значения и его заполнение (обнуление в данном случае) лучше отделить от логики, вынеся в отдельный класс-объект-что угодно.

Дальше.
function addTodo(field) { /* */ }

Тут, конечно, жесть полная. Помимо ошибок, которые я уже поминал, сама идея генерировать элементы в коде не через шаблон ужасна. Если ну никак не хочется использовать шаблонизатор, даже работающий на простом replace, можно уж прибегнуть к cloneNode, подменяя в нем значение в тех элементах, которые помечены как data-text, data-delete и так далее. Но лучше все же смотреть в сторону шаблонизатора.

function allCheckList() { /* ... */ }

Бегать по дереву элементов и считать сколько элементов, чтобы показать сумму -- хитро, я бы не додумался. Только что, если появится пагинация или фильтрация?

И, как и обещал, про скоуп приложения. Как я где-то прочитал
Люк, никогда не гадь в глобальный скоуп!

Следует обернуть весь код в самовыплняющуюся функцию с целью избежания конфликтов чего-либо еще на этой странице и не выпускать из области видимости этой функции ничего кроме непосредственного API этой функции.

Ну и теперь самое важное.

Если задача звучит так: сделать на vanilajs todo-лист на коленках за 5 секунд и навсегда забыть о нем, даже не правя баги в нем -- да нормальное решение, можно катить в прод, исправив проверку на пустоту интпута. Если же задача сделать что-то качественное, поддерживаемое и масштабируемое -- все в корне не верное. Тут дело даже не в выборе библиотеки, фреймворка или чего бы там ни было. Тут просто максимально простое влоб решение, которое обернется невероятным количеством геммороя при дальнейшей разработки. И речь не о тех мелочах, до которых я придирался выше -- это все не так важно. Гораздо важнее что тут нет никакой методологии, правильного подхода, паттерна и это является фатальным недостатком. Как его исправить?

Ну не вдаваясь в подробности о теории программирования, что в идеале нужно знать, хотя бы понимать о чем они, оба подхода, вариантов два -- либо ООП, либо функциональщина. Конечно, можно оставить все на процедурах, но именно процедурный подход изжевает себя примерно на 1000 строк кода полностью, на js даже быстрее.

Выбрав путь ООП дальше следует обратить внимание на семейство паттернов MVC, слабую связанность, модульность и много там всего, намека на что у вас нет. Собственно, создаете класс Todo, потом TodoList, который будет списком экземпляров Todo, потом контроллер, который как раз и будет проверять введено ли что-то валидное в поле ввода, потом View, который как раз и свяжет шаблон с данными, потом возникнет вопрос -- кто должен создавать экземпляр Todo, потом откроете для себя абстрактную фабрику, потом закроете ее обратно, покуда в js она никому не упала, в общем -- об MVC статей написано больше, чем о всем остальном программировании вместе взятом.

Выбрав путь функционального программирования сперва появится некое хранилищи структур Todo, появится несколько чистых функций, которые будут изменять это хранилище и обрабатывать различные варианты, так же вынесется отдельно слой View, откроется паттер Observer (ну или как ныне более модно говорить -- реактивное программирование) - об этом тоже информации много.

P.S. Счетчик активных туду, к слову, не пересчитывается при добавлении нового. Не зачет, в прод не идет.
Ответ написан
Пригласить эксперта
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Похожие вопросы
Rocket Смоленск
от 80 000 до 130 000 ₽
div. Ставрополь
от 40 000 до 90 000 ₽
Wanted. Санкт-Петербург
До 220 000 ₽