Публичная порка JS-гуру
Конкурс закончился и работы проверены. Работы прислали 11 программистов, из которых один участник прислал аж 8 вариантов.
На удивление решения задачи оказались очень разные. Многие писали обработчики событий обычным способом, но некоторые всё-таки рекомендуемым AddEventListener/AttachEvent. Поразило разнообразие способов проверки на пробелы и наличие классов.
В принципе, поскольку сама задача была решена практически всеми, то нет смысла опираться на эти результаты при подведении итогов. Поэтому, я буду придираться не только к самой задаче, но и к стилю программирования.
Said
Эффективное решение с помощью цикла проверить все поля и свойства объекта inputData перед выводом. А, ведь действительно, поля могут быть заполнены не только вручную, но и скриптом.
Хорошее решение вынести все тримы в отдельные функции и использовать их как в PHP. Правда, на мой взгляд такие вещи должны прописываться как прототипы.
Нехороший повтор AddEventListener/AttachEvent. Можно было объединить.
Неправильное изменение элемента. Класс должен добавляться, а не заменяться.
Не могу понять эту конструкцию.
if (typeof(e.target) == 'text') e.target.className = 'edit';
Разве typeof(e.target) должен возвращать text? По-моему он возвращает object. Соответственно эта часть не работает в Firefox. Видимо автор скрипта хотел проверить тип элемента формы. Правильно было бы написать так:
if (typeof(e.target) != 'undefined' && e.target.type == 'text') e.target.className += ' edit';
Правильно, добавил проверку существования свойства объекта inputData, на тот случай если в форму добавили поле ввода, а свойство в inputData не добавили.
if (!!inputData[el.name] || inputData[el.name] == '') inputData[el.name] = el.value;
Только для чего написаны два восклицательных знака? Ведь в конструкции if такие вещи в любом случае конвертируются в Boolean.
SergiusD
Хорошая работа с массивами.
Есть проверка существования класса error.
Извращение:
arr[ind] = null;
Для таких вещей есть функция splice.
Неоправданное решение относительно поиска в className, путём создания прототипа indexOf. Даже если использовать этот вариант, то лучше было бы разбивать строку в массив уже в indexOf. А возвращать и проверять в этом случае правильнее не -1 и ещё какое-то число, а true и false.
Андрей
Сделано не совсем универсально. При убирании фокуса возвращается первоначальный класс как первый. А если их там несколько?
Примитивный и неоптимизированный вывод данных в showAll. Гораздо лучше можно было бы сделать и не использовать лишние переменные.
Такие вещи нужно объединять в единое регулярное выражение:
.replace(/^\s/, '')
.replace(/\s$/, '')
Николай
Хороший вариант обращения к объектам по id или напрямую. Любят такие вещи во фреймворках.
var $ = function(id) {
return typeof id == 'string' ? document.getElementById(id) : id;
};
Смысла особого нет в том, чтобы обращаться к объекту не на прямую, а через $, но ради единообразия…
Изящный вариант проверки существования класса:
return !!el.className.match(new RegExp('(^|\\s)' + className + '(\\s|$)'));
Мусор в цикле:
l = inputs.length; i < l
Это зачем?
})(inputs[i]);
Игорь Источник
Параноидально концептуальный подход. Годен для больших проектов. В пору писать свой фреймворк.
Хорошая прописка прототипов, особенно ifEmpty.
В конце вывода лишний перенос строки.
Если удалить текст, то поле становится красным. Если дать ему фокус, то класс будет: read error edit.
element.className = element.className + " " + className;
для таких вещей есть оператор «+=»:
element.className += " " + className;
Тяжеловатый для восприятия стиль.
FX Poster
Да, с классами лучше всего работать как с масивом, а не как со строкой. Правильно.
С пробелами так работать нельзя:
this.value = this.value.replace(/^ +| +$/g, '').replace(/ +/g, ' ');
Нужно искать пробельный символ а не пробел.
Зачем столько строгих проверок (===, !==)?
В конце вывода будет лишний перенос строки.
Артём Глувчинский
Тоже хороший вариант проверки существования класса
if (this.className.indexOf(errorClass) >= 0)
В задании сказано «очистить содержимое поля от лишних пробелов». Значение поля очистил, но не заменил содержимое на очищенное значение.
В функции getResults после вывода сообщения об ошибке можно просто написать return а не return false?
Артём Макаров
Не убирается последний пробел в поле ввода.
В регулярном выражении указан пробел, а не пробельный символ.
Некрасивая обработка классов удалением слов и пробелов.
Хитро:
response += (response.length ? "\n" : '') + v + " = " + inputData[v];
но запись в массив и объединение с помощью метода join() — умнее.
Плюс в том, что когда необходимо быстро написать код, такое последовательное решение наиболее удобно в восприятии.
Dmytro-Shteflyuk
Чем больше щёлкаешь по полю, стираешь и вписываешь текст в поле, тем больше пробелов у поля ввода между read и edit.
Чем такой вариант:
results.push(name);
results.push('=');
results.push(inputData[name]);
results.push("\n");
лучше чем
results.push(name + '=' + inputData[name] + '\n');
?
Преимуществ не вижу. Опять же в конце будет лишний перенос строки.
Алик Кириллович
Победитель, который не только правильно выполнил задание, но также исследовал поимку событий в случае динамического добавления полей, а так же переноса введённых данных из одного поля в другое (drag&drop). Гостевой пост от победителя, в котором раскроются все секреты исследования ожидается в ближайшее время.
И последнее
Господа программисты, я задолбался конвертировать эти каракули:
Ýòà ôóíêöèÿ óñòàíàâëèâàåò îáðàáîò÷èê fncAction ñîáûòèÿ txtType äëÿ óçëà, óäîâëåòâîðÿþùåãî óñëîâèþ fncCondition.
Выучите наизусть, раз и на всегда!
UTF-8
Я очень рад, что мои читатели откликнулись на предложение и приняли участие в конкурсе, но я никак не думал, что их будет так много! Проверять одному достаточно сложно, поскольку со временем глаз замыливается и можно пропустить очевидные ошибки, а в других работах они могут броситься в глаза. Нужно было бы собрать целую комиссию для объективной оценки работ. Поэтому прошу быть снисходительными к результатам конкурса. Думаю, что мои замечания для вас будут только полезными и не вызовут агрессивного настроя: «Ууу, докопался!»
Спасибо всем за участие!
]]>Evgeny Sergeev]]>>Неоправданное решение относительно поиска в className, путём создания прототипа indexOf. Даже
>если использовать этот вариант, то лучше было бы разбивать строку в массив уже в indexOf. А
>возвращать и проверять в этом случае правильнее не -1 и ещё какое-то число, а true и false.
Мне кажется, что функция indexOf сделано вполне грамотно, другой вопрос, что использована неправильно. А Ваш совет лишен для меня всякого смысла. Если бы функция возвращала true или false, то она называлась бы inArray! Так же непонятен тайный смысл разбиения строки внутри метода. Где логика?
»]]>Evgeny Sergeev]]>>но запись в массив и объединение с помощью метода join() — умнее.
Почему?
»]]>Артём Макаров]]>Ууу, докопался!
Прошу прощения, но я не Марков, а Макаров.
»]]>Никита]]>Evgeny Sergeev, если говорить о логике, то название inArray как раз таки отражала бы суть функции. Но если подходить к вопросу ещё логичнее, то тут должно быть две функции: hasClass и inArray. Первая должна применяться к строковому значению element.className и занималась бы разбиением строки по пробелу, а вторая бы вызывалась из первой, чтобы найти соответствующий класс в массиве.
Запись в массив умнее, поскольку гораздо удобнее одной функцией соединить все элементы массива проложив между ними определённый символ. В этом примере, каждый раз при дописывании строки будет проверяться длина response. Зачем это делать если толк от этого в цикле единовременный?
Артём Макаров, извини, поправил.
»]]>Игорь]]>Большое спасибо за обзор
Осмелюсь немного прокоментировать свои огрехи 
>Параноидально концептуальный подход. Годен для больших проектов. В пору писать свой фреймворк.
Угу, есть такое дело, в последнее время только большими проектами и занимаюсь, оттудава и подход
>Хорошая прописка прототипов, особенно ifEmpty.
Спасибо
привычка осталась со времен искользования ф-ей empty в PHP 
>В конце вывода лишний перенос строки.
Решил, что это не имеет значения, поэтому и не обработал.
>Если удалить текст, то поле становится красным. Если дать ему фокус, то класс будет: read error edit.
Делал так специально. Ибо довольно часто вижу в реальных проектах, что красная подсветка остается при вводе нового значения + сам всегда использовал такой же принцип, к тому же воппринял слово “добавить” в формулировке задания буквально.
>element.className = element.className + ” ” + className;
>для таких вещей есть оператор «+=»:
>element.className += ” ” + className;
Согласен, обычно пишу именно “+=”, не помню, почему написал именно так
>Тяжеловатый для восприятия стиль.
Угу, тоже согласен, стиль “не жаваскриптовый”, поэтому и воспринимать сложно
»]]>[NSU]said]]>О, наконец-то результаты!
Прокомментирую один момент:
“if (typeof(e.target) != ‘undefined’ && e.target.type == ‘text’)” – я так же писал поначалу, но потом, не проверив во всех браузерах, написал “if (typeof(e.target)==’text’)” и оно заработало. Лучше бы не пытался хитрить
В целом приятно, что на меня потратили время, и указали на мои ошибки. Среди знакомых сложно найти критиков в этой области такого уровня.
P.S. Сел изучать чужие решения.
»]]>Андрей]]>Ууу, докопался! А если честно, ОГРОМНОЕ спасибо за то, что потратил на это время. Теперь буду знать как выводить массив данных )
»]]>Octane]]>Работа автора Daniil Loshkarev
»form = document.all.my_form;Такая конструкция не работает у меня ни в одном браузере
Строка из FireBug:
document.all has no properties[Break on this error] form = document.all.my_form;
]]>Никита]]>Octane, эта работа вообще не принята. Видите, в обзоре его нет.
»]]>Octane]]>Просто интересно))) у автора же работало в каком то браузере
я проверил в IE5.5|6|7|8b1, FF2.0.0.14, Opear 9.27|9.5b2, Safari 3.1(win) и нигде не заработало…
»]]>pepelsbey]]>> l = inputs.length; i < l
А мне казалось, что это не мусор, а экономия на подсчёте длины массива во время каждой итерации. Разве нет?
»]]>Игорь]]>pepelsbey, такие вещи желательно писать отдельно, а не пихать все в один оператор, поэтому и “мусор”
»]]>excieve]]>Спасибо за обзор ошибок. Буду изучать чужие работы.
Только вы перепутали мою фамилию:)
Артём Глуховский -> Артём Глувчинский
Теперь касательно “докопался”:)
“В функции getResults после вывода сообщения об ошибке можно просто написать return а не return false?”
Ну по-моему это не суть важно. Поправьте, если я не прав.
“В задании сказано «очистить содержимое поля от лишних пробелов». Значение поля очистил, но не заменил содержимое на очищенное значение.”
Действительно, не совсем понял задачу. Подумал, что достаточно вывести очищенные значения в “Show All”.
В любом случае, спасибо!
»]]>Evgeny Sergeev]]>Никита,
Вот видите, сами соглашаетесь, а советуете пихать всякую ерунду в indexOf.
»]]>lusever]]>Что-то я не понял про indexOf, это однозначно плохое решение для поиска класса.
»‘myclassname2′.className.indexOf(‘class’) => 2
]]>Kolyaj]]>> pepelsbey, такие вещи желательно писать отдельно,
>а не пихать все в один оператор, поэтому и “мусор”
Такие вещи относятся к инициализации цикла, поэтому и пишутся в заголовке цикла.
> Это зачем?
>})(inputs[i]);
Это замыкание, без которого при таком подходе никак.
> Смысла особого нет в том, чтобы обращаться к объекту
> не на прямую, а через $, но ради единообразия
Смысл в таком виде функции $ в том, что удобно писать функции, принимающие как id элемента, так и сам элемент.
> Тоже хороший вариант проверки существования класса
> if (this.className.indexOf(errorClass) >= 0)
Вы меня пугаете, вы правда считаете это хорошим вариатном?
> И последнее
»> Господа программисты, я задолбался конвертировать эти каракули
Если вам несколько программистов прислали код в кодировке win1251, а ваш редактор не может ее распознать, так может дело не в программистах, а в редакторе?
]]>Evgeny Sergeev]]>lusever, речь идет не о String.indexOf, а о Array.indexOf котрую сделал SergiusD
»]]>Никита]]>excieve, мда, что-то у меня с фамилиями… исправил ))
Evgeny Sergeev, не понял, в чём противоречие?
Kolyaj,
> Такие вещи относятся к инициализации цикла, поэтому и пишутся в заголовке цикла.
в данном контексе относятся, но чаще всего длина массива используется и в других местах кроме цикла.
> Это замыкание, без которого при таком подходе никак.
Чем оправдан такой подход?
> Смысл в таком виде функции $ в том, что удобно писать функции, принимающие как id элемента, так и сам элемент.
Зачем обращаться к элементу через $ если можно без неё?
> Вы меня пугаете, вы правда считаете это хорошим вариатном?
пересмотрел, уже не считаю. Действительно способ неудачный, ибо может возвращать истину, если будет найдены классы начинающиеся на «error».
> Если вам несколько программистов прислали код в кодировке win1251, а ваш редактор не может ее распознать, так может дело не в программистах, а в редакторе?
»начнём с того, что не win1251, а win1252. И это бывает только тогда, когда в win-системе, на которой создавался код установлен русский язык для неюникодовских программ.
]]>Kolyaj]]>> но чаще всего лина массива используется и в других местах кроме цикла.
Чаще они как раз не используются в других местах. Чаще нам надо просто получить и перебрать элементы.
> Зачем обращаться к элементу через $ если можно без неё?
»Можно писать функции вида:
funciton addClass(element, class) {
element = $(element);
…
}
и не заботиться о том, что нам передали: id или сам элемент.
]]>Kolyaj]]>> Чем оправдан такой подход?
»Это используется в случаях, когда callback-функции создаются внутри цикла (счетчик цикла будет иметь другое значение на момент вызова callback). В данном случае, конечно, можно было все замкнуть на this, но мне так привычнее.
]]>Никита]]>Kolyaj, в таком случае нужно функцию вызывать так:
addClass($(element), ‘edit’);
чтобы в каждой вами созданной функции не писать element = $(element)
Раз уж вы так дотошны, тогда позвольте произвести ещё один пинок:
»inputData[input.id] = input.value;
Обращение к этим элементам нужно производить по name, а не id.
]]>Kolyaj]]>> Kolyaj, в таком случае нужно функцию вызывать так:
> addClass($(element), ‘edit’);
Ну мы же в первую очередь хотим, чтобы функцией было удобно пользоваться, а не чтобы ее было удобно писать.
> Раз уж вы так дотошны
»Просто отвечаю на вопросы, возникшие по моему варианту. Про utf уж слишком крупно было. А пинок да, логичный.
]]>Evgeny Sergeev]]>Никита, противоречие в том, что Вы предлагаете в функции indexOf написать код, который не соответствует ее назначению.
»]]>Evgeny Sergeev]]>> Смысл в таком виде функции $ в том, что удобно писать функции, принимающие как id элемента, так и сам элемент.
>>Зачем обращаться к элементу через $ если можно без неё?
Из серии зачем писать на JavaScript, если на Flash будет красивее.
»]]>Никита]]>Evgeny Sergeev, какой код я предлагал написать?
> Из серии зачем писать на JavaScript, если на Flash будет красивее.
»не надо цепляться к словам, я просил объяснить причину и мне её объяснили. Дочитывайте диалог до конца.