Креш-тест: оценка уровня написания JavaScript-кода
— Оцените, пожалуйста, уровень написания кода — demenkov.dp.ua/dou.
Что хорошо, что плохо, и в каком направлении сосредоточить усилия по повышению своей квалификации?
Я почитал код и в целом, нужно сказать, всë довольно неплохо. Т.е. все минусы, которые я пишу ниже, это не какие-то глобальные проблемы, а разные мелочи.
Очевидно, что по трём таким примерам оценить уровень мне сложно, но если бы я такое увидел на собеседовании, я бы скорее всего человека взял ;)
Итак, по минусам:
- Это не имеет отношения к именно скиллу программирования, но всë же. Стилистика хромает. То есть пробелы, то их нет: в перечислении аргументов функции, после слов if/for, при присвоении, и т.п. Это раздражает, если честно, несколько тяжело читать конструкции «for(i=0;i<bodychilds.length;i++)». Ширина строк — до бесконечности. Это уже вопрос такой, мы у себя держит не шире 80 символов, кто-то ставит 120, но всë равно, нет смысла писать очень много в одну строку. В первом файле (остальные не проверял на это) смешаны одновременно табы и пробелы для отступов.
- Строки, как управляющие элементы — это неправильно. В них можно ошибиться и никогда этого не узнаешь. Конкретное место — в первом примере функция manipulateBlock. Если так хочется открывать/закрывать одной функцией, можно было сделать параметр-boolean, или что-нибудь такое. Или chartMode. Я бы выделил из drawData какой-то хелпер и сделал бы две функции, одну для рисования линий, другую — для гистограмм.
- Тот же manipulateBlock. Цикл for идëт по всем дивам, несмотря на то, что нужный существует всего один. Не проще ли было сделать функцию getBlock, которая берëт айдишник и по нему находит и возвращает блок? Тогда и смысла в одной функции на сохранение и удаление не остаëтся. Более того, можно было id ложить не в атрибут idblock (который, кстати, не существует, для них рекомендуют использовать атрибуты data-*), а просто в id — и тогда getElementById, и дело в шляпе.
- Один хендлер на все ивенты — спорное решение, и более того, onclick? addEventListener же.
- Маленькая (совсем) ошибка проектирования. Во втором примере getData, если это будет аяксовая функция, не сможет ничего возвращать. Ей нужен коллбэк.
- Опять поиск таблицы (в 3 примере) с помощью цикла. getElementsByTagName("table")[0]?
- Не понимаю, зачем постоянно использовать worker.что-то, если можно this.что-то?
Фух, закончил вроде. В целом всë клëво, я довольно придирчивый, мне кажется. По крайней мере, у нас в команде я очевидно самый придирчивый, так что не напрягайся сильно.
Мой вариант решения задачи № 1
Он работает идентично оригинальному. Я не менял стили/HTML (практически), только заменил JavaScript.Я выделил отдельно функции работы с меню, блоками, и просто общие хелперы. Это упрощает тестирование (можно тестировать каждый уровень функциональности отдельно), и понимание — легче понять, кто за что отвечает.
Например, уровень обработки блоков ничего не знает про меню и не в курсе про структуру DOM, меню спокойно себе тасует данные и почти не в курсе про блоки (ну, немного в курсе, конечно ;)).
Один из ивент-хендлеров делает немного больше, чем я хотел бы (аппендит блок к родительскому элементу), но при таком количестве кода я не вижу смысла про это много думать. Ну и в целом я постарался организовать/отформатировать код так, чтоб это было легко читать.
Можно было бы, кстати, завернуть блоки в объекты, и т. д., но большого смысла это не несëт. Тут не надо менеджить слишком сложные структуры и не предполагается развития кода, поэтому хранение данных в DOM вполне нормально работает.
Удачи!
P.S. И английский нужно импрувить, если надписи во втором примере — не опечатки/etc. О чëм ты знаешь, наверное.
Все про українське ІТ в телеграмі — підписуйтеся на канал DOU
11 коментарів
Підписатись на коментаріВідписатись від коментарів Коментарі можуть залишати тільки користувачі з підтвердженими акаунтами.