Pull to refresh

Comments 203

Я встречал код

if Date()>«20.01.2001»
messagebox(«Неизвестная ошибка»);
exit;

Синтаксис точно повторить не смогу, это было на VB версии, устаревшей уже на тот момент.
ПО Считало в банке какие-то показатели для отчётности.

Человек, писавший этот код, уволился в декабре 2000 года.
Я как-то видел код банковского программиста, который наоборот, не хотел увольняться, и сделал всё, чтобы быть незаменимым. Его софтина была в одном файле из полутора сотен тысяч строк, и она была обфусцирована. Не в полной мере, конечно, но переменные звались исключительно a, b, c, d..., функции тоже в таком духе. Это было сделано, чтобы никто другой не смог этот проект развивать. И да, мы действительно переписали эту гадость с нуля.

Вопрос в том, как он это поддерживал.

открывал необфусцированный репозиторий, делал изменения, обфусцировал, выкладывал в рабочий репозиторий?

В 2000 году он мог обходиться и без репозиториев ;). Me своими глазами видел код, в котором все таблицы были mXXX.dbf, поля в них Ryyy… Те самые годы, foxpro, 3 программиста, и никаких репозиториев.

Когда смотрел, как 1С хранит данные в БД… Это были такого же рода таблицы и метаинформация в одной из таблиц, с маппингом этой мути на объекты в 1С
Чтобы быть точным, в 1С 7.7, метаинформация содержится не в таблицах, а в файле 1cv7.md, в 1С 8.x перенесли в БД, но IMHO используется «VFS». Всё равно «файлы» внутри СУБД.
Наверное, у него был файлик, где всё было расписано, в том числе и назначение каждой переменной
В студенческие годы я писал одну программу расчета, которая в зависимости от комбинации пары десятков чекбоксов и листбоксов брала данные из бд, и выводила некий результат. Кроме того, что приложение было написано в классическом стиле button_logic, я даже не переименовывал элементы интерфейса, и в коде была натуральная портянка вида:
if (checkbox1.checked and checkbox8.checked)
listbox9.enabled=false;
и что-то типа:
if (len(textbox19.text)>0)
sql=sql+" and column='"+textbox19.text+"'"; //инъекции, не не слышали, тогда

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

Так что возможна ситуация, когда программист даже специально и не обфусцировал ничего, он просто и так помнит какая переменная и функции за что отвечают.
Это стиль кода примерно так 95% ПО тех лет, когда появился Delphi сотоварищи.
Быстренько «набросать на форму» сотню-другую контролов, далее столько же событий и после этого героически о всем этим бороться.
Не только Delphi, аналогичное есть и в Windows Forms и qt

Ну, в студенческих программах и я помнил предназначение переменных :) Но речь о 150k loc. У меня на прошлой работе был проект примерно такого размера (большинство которого, правда, я сам не писал), то за 2.5 года я все ещё не во все части кода успел залезть, не то чтобы запомнить предназначение всех вещей. При том, что проект написан был сравнительно грамотно, не «все-в-одном-файле-с-рандомными-именами».
С другой стороны, смотря на исходники того же телеграма для Android и что оно до сих пор в состоянии развиваться, я даже котов поверить, что если человек работает над проектом с самого начала, он может плюс-минус помнить то, что он писал.

Не глумитесь над животными. Не надо поверять котов.

Не годится. Софтина была как раз на Паскале, точнее, на Delphi :)
Собственно, как-то поддерживал. Я не думаю, что у него где-то был деобфусцированный текст, скорее всего, просто помнил. Когда проект всего один, и ты туда каждодневно залезаешь, то в принципе все равно будешь помнить, где там какой код что делает.
Язык не имеет особого значения: как написано в той же самой статье по ссылке «закоренелый настоящий программист может написать фортрановскую про-
грамму на любом языке.»
Да, он наверняка помнил. А если что и не помнил — то смотрел в коде: «настоящие программисты не нуждаются в комментариях: текст программы все объясняет»

Самодокументируемый код же :)

Там и не нужно было ничего помнить. Это было так круто… Жмакаешь на поле ввода, выбираешь справа обработчик, и после даблклика попадаешь в его текст. А как называется компонент — последнее дело… :)
UFO just landed and posted this here

Я работал с таким человеком. Раньше он в университете преподавал.


Выход за границы массива? Да нее, просто увеличу размер в 10 раз на всякий случай.
Локальные переменные? Пфф, глобальных хватит. Да ещё и объявлю их с запасом. А то в начало файла лазить не хочется каждый раз, ведь тут полторы тысячи строк. В которых объявлено 3 процедуры, которые используют исключительно глобальные переменные.


Ребят, мне нужно читать изображения, но я не умею. Присылайте в текстовом виде размер и R,G,B в каждой строчке.

Да нее, просто увеличу размер в 10 раз на всякий случай.

Это же легендарный «хвостик». Есть даже преподы, которые этому УЧАТ.
Я не так давно тоже исправлял такую «неизвестную ошибку», код один в один. Человек, писавший этот код, не увольнялся, а просто взял и умер
Возможно, он обладал удивительными способностями…

На самом деле я на заре развития ИТ в стране часто видел такие подходы.
С каждой новой версией дата отодвигалась и всё хорошо работало, но когда заказчик пытался выйти из отношений с разрабом, оно вылазило.

И хорошо если так — когда есть текст сообщения, который по коду находится на раз, два.
Каждый раз, когда я вижу «неизвестную ошибку», я либо серьёзно сомневаюсь в способностях того, кто написал выдающий её код, либо читаю это как «ошибка, об истинной природе которой вам знать не положено». У нормальных программистов «неизвестных ошибок», всё-таки, не бывает.
UFO just landed and posted this here
Ну ошибку же что-то вызвало? Была же какая-то проверка, в результате которой была обнаружена ошибка? У этой проверки же была какая-то цель? Значит, и хоть сколько-нибудь осмысленное сообщение об ошибке сформулировать не должно составить труда.
Была же какая-то проверка, в результате которой была обнаружена ошибка?

Очевидно, была — какой-то глобальный обработчик исключений в программе, ну или локальный обработчик вокруг крупного блока кода, дифференцировать все возможные исключения в котором программисту было некогда или просто лень. Это вполне себе типовой кейс. Ну а «Неизвестная ошибка» вместо технических подробностей, это просто чтобы не взорвать мозг пользователю, вываливая на него содержимое стека вызовов.
Так зачем дифференцировать? Кинуть своё с общим сообщением об ошибке, добавив к нему подробное из пойманного. В джаве, например, это поведение вообще встроено, можно передать в конструктор исключения другой Throwable в качестве причины, и во всяких логах выведется соответствующим образом.

это просто чтобы не взорвать мозг пользователю, вываливая на него содержимое стека вызовов
Ну так всё равно можно же вывести какое-то общее сообщение об ошибке. «Ошибка при чтении файла», «ошибка сети», «ошибка ввода/вывода», «внутренняя ошибка на сервере», и так далее. И мозг не взорвал, и совесть чиста :)
В джаве там обработка исключений контролируется ещё компилятором, там при желании забить на это не выйдет.
Ну так всё равно можно же вывести какое-то общее сообщение об ошибке. «Ошибка при чтении файла», «ошибка сети», «ошибка ввода/вывода», «внутренняя ошибка на сервере»

Ну так это же надо ещё сесть и тестировать, тестировать, тестировать… Эти «неизвестные ошибки» пишут же не тогда, когда там сразу понятно, какой тип исключения вылез, а когда их надо ещё выявить.

Это ж 4 catch надо делать

Есть как минимум 2 способа объединить их в один. ИМХО, делать раздельные catch для разных типов исключений имеет смысл только если код для их обработки должен быть разным.

Можно перечислить типы (новая фича в 8й джаве):
try{
    ...
}catch(IOException|IllegalArgumentException|SomeCustomException x){
    throw new RuntimeException("Failed to do something", x);
}

Можно просто ловить любой из базовых классов:
try{
    ...
}catch(Exception x){ // или Throwable
    throw new RuntimeException("Failed to do something", x);
}
Можно перечислить типы (новая фича в 8й джаве):

Во-первых, эти типы нужно знать. Во-вторых, нужно, чтобы это была 8-я джава. Ну, или 6-й С#
Можно просто ловить любой из базовых классов:

Ну а это и есть та же самая «Неизвестная ошибка»
Ну а это и есть та же самая «Неизвестная ошибка»
Так нет — в этот try-catch же обёрнуто какое-то действие, верно? Это «ошибка при %название_действия%», с подробностями внутри, если они нужны, включая все стектрейсы.
Так нет — в этот try-catch же обёрнуто какое-то действие, верно?

Мы же говорим про
try{
    DoDohrenaRaznogoStuff();
}catch(Exception x){ // или Throwable
    throw new RuntimeException("Failed to do something", x);
}


Если известно конкретное действие, то и «неизвестных ошибок» не бывает.
Ну так у этого исключения всё равно будет более подробное внутри. Если вызвать printStackTrace(), оно выведет его в виде «caused by: java.lang.IllegalArgumentException: очень конкретная ошибка». По-моему очень удобно.

Есть небезосновательное мнение, что checked exceptions в Java — это зло. От неправильного try-catch гораздо больше вреда, чем если бы его вообще не ставить — в последнем случае все-равно где-нибудь выше сработает обработчик, поставленный умными и компетентными людьми. Сколько встречал дерьма наподобие:


try { ... } catch (Exception e) {
  log.error(e.getMessage());
}

или даже


try { ... } catch (Exception e) {
  throw new MyCustomException("Error: " + e.getMessage());
}

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

Если объединять, то и будет "неизвестная ошибка"


Вообще сарказм был.

Ошибка может быть обрабатываемой пользователем — «Не указана фамилия», или не обрабатываемой им — «SQL ERROR...». В первом случае нужно хорошо обработать, показать ему вменяемое короткое сообщение, после которого он поймёт, в чём косяк, а во втором ему не нужно сразу ничего показывать. И просто сказать «Унимание, вохгткла ошибка где-то внутри. Сохраняй спокойствие и сообщи в службу поддержки её код ХХХ..».
И тут абсолютно всё равно, во время чего произошла ошибка. «Ошибка при сохранении файла» — самое дурацкое сообщение. Что делать то, если оно вылезет?
Для таких случаев мне нравился подход в Делфи 5..7 (примерно)
Они выводили сообщения: «Исключение #...»
Т.е. если в каком то месте кода мы считаем, что возникновение ошибки маловероятно, мы можем не обрабатывать её, но бросить уникальное исключение (и, например, логгировать его) чтобы потом можно было определить место, где обнаружена ошибка.
В норме там будет Eurekalog, который создаст .elf файл с callstack'ом и местом возникновения ошибки, вплоть до строки. Ну или Jcl Debug.
Ну и да, чем выкидывать ошибку с номером, уж лучше сделать Raise Exception.Create('Exception: Module/Methodname: SomeUseful disagnostic msg').

Vlad_IT
В коде автора цикл завершится сразу, как найдется пользователь. В вашем коде цикл будет проходить все элементы всегда.

Не пользователь, а пользователь с правильным паролем, НЯП. Если пароль неверный, он же до конца таблицу досмотрит, не?

Зато одному пользователю можно иметь сразу пачку паролей!

Вообще, это подход из серии надписи на приборе «внутри нет частей, которые вы могли бы починить».
Реальный случай. Приложение, написанное отделом А, решает положить введённые пользователем данные в базу, написанную отделом Б. По требованию заказчика, база иногда внезапно и нещадно изменяется — а новая версия программы разворачивается с опозданием. Так что случается, что сервер СУБД даёт программе неожиданный и необрабатываемый отлуп из серии «нет такой таблицы в этой базе».

Дать пользователю текст ошибки от сервера СУБД? Пользователь не имеет ни прав лезть в базу, ни знаний для этого. Что ему делать с этой безусловно полезной информацией?
Всё, что ему остаётся, увидев ЛЮБУЮ ошибку (не вызванную его действиями) — это «Halt and catch fire». Он прекратит работу и позовёт администратора.

Потому программа сейчас выводит скромное сообщение из серии «При сохранении данных возникла неизвестная ошибка взаимодействия с сервером. Обратитесь к разработчику».
А полный текст ошибки, конечно, попадает в лог для команды разбора.
Хм. У меня в коде есть WTF ошибки.
Опыт научил меня, что надо обрабатывать все случаи, но проблема в том, что некоторые случаи невозможны. Но я всё равно ставлю туда лог с текстом в духе «WTF! Impossible error.». Занимает такая проверка не очень много, зато в логе всегда есть какая-то инфа.
И да, такие обработчики иногда срабатывают. Например достаточно часто инкрементальная сборка некоторых проектов начинает косячить и не пересобирает объектники, которые надо собирать. В итоге едут указатели и «невозможная» ошибка становится возможной. Для меня появление такой ошибки в логе первый признак того, что проект был собран не правильно. Но написать туда что-то вроде «Incorrect build error» я не могу, потому что это лишь одна из возможных причин срабатывания обработчика. И даже написать «Maybe incorrect build» я не могу тоже, потому что поехавший объектиник это UB и предполагать о том, что это UB может вызвать какой-то не валидный код я не могу.
UFO just landed and posted this here
Я использую вывод «Неизвестная ошибка» там, где юзверь делает те действия, которые ему делать не положено

Честно говоря, в таком случае проблема не в юзвере, а в вас. В нормально спроектированной программе не должно быть доступно кнопок, которые нельзя нажимать.
UFO just landed and posted this here
Никогда не сталкивались с попытками заведомого ввода некорректной информации при регистрации или просто ввода данных, например? :) Не случайными ошибками, а именно спланированными?

Сталкивался. Но ведь программа в этом случае должна делать валидацию входных данных, и сообщать, что не так. Если важно контролировать, что юзеры пытаются обмануть систему, то логировать подобные попытки. Но уж точно не выдавать «неизвестные ошибки».
UFO just landed and posted this here
Но в данном случае пользователю НЕ НАДО сообщать, что конкретно он делает не так, потому что это может облегчить ему задачу.

Мне сложно представить себе кем-либо специально выстроенный бизнес-процесс, в котором у вас есть вредитель (условно вредитель, пусть даже просто юзер, который показательно кладёт большой болт на служебные инструкции), у него достаточные права для того, чтобы осознанно вредить, а препятствием на пути у него является программа, которая просто не говорит ему, что он сделал не так. Наоборот, вы только укрепили у меня уверенность, что что-то у вас там организовано неправильно.
Представитель конкурента, купивший у вас ПО для натурально выглядящего отчёта «как вообще этим пользуются»

Что Вы предлагаете возвращать пользователю в случае инфраструктурных исключений? Возвращать сообщение самого исключения значит раскрывать конкретные подробности своей инфраструктуры, которые пользователю знать нет никакой необходимости, уже не говоря о том, что это потенциальная дыра для сбора информации хакером. А какое-то сообщение вернуть надо (элементарное уважение к пользователю) — например, что-то вроде "Ошибка приложения, обратитесь в техподдержку по такому-то номеру". Вот это "ошибка приложения", имхо, эквивалентно "неизвестной ошибке", хотя я бы предпочёл первое.

Что Вы предлагаете возвращать пользователю в случае инфраструктурных исключений?

У всех у них есть какая-то классификация. Или как минимум, разделение на три большие категории «может пройти сама», «может быть исправлена пользователем», «требует обращения в ИТ-саппорт». Для первой надо выдавать сообщение, например, «Удаленный сервер в настоящее время недоступен, повторите запрос позже», для второй, например, «Введенные данные не соответствуют требованиям к оформлению входящих документов», для третьей, как вы и написали, «В приложении произошёл внутренний сбой, обратитесь в техническую поддержку». Раскрывать ему стек вызовов, естественно, не нужно, но вот у технической поддержки возможность достать подробности исключения должна быть. И вполне логично, если он будет там где-то писаться, даже на машине юзера, если подобные сбои могут предполагать отсутствие возможности писать куда-то в удалённый лог на сервере.
UFO just landed and posted this here
Я никуда не уходил. Я просто объяснил, что у вас творится какая-то фигня в организации. И если бы у вас было более разумное объяснение этому, кроме как «да так само сложилось, просто никто не думал, как лучше», вы бы написали :)
UFO just landed and posted this here
У него программы наверняка сообщают: «При вводе пароля вы ошиблись в третьем символе. Там должно быть А вместо а. А логин вообще User20398»
UFO just landed and posted this here
Боюсь, члены жюри в отборочном туре могут подраться между собой, решая, кого из вас двоих отправить на финал конкурса юмора имени Петросяна.

Вы будете смеяться, но помнится какой-то BIOS был так запаролен: нажимаешь букву, если правилно, то звездочка появляется и курсор передвигается дальше. Если нет, то пищит и ничего не происходит.

А я вам на него написал конкретный ответ: такого быть не должно в принципе. Если юзер нечаянно вводит неверные данные, программа должна на это указать. Если юзер сознательно вводит неверные данные, ему начальство должно дать по шапке, а дело программы — залогировать это, если нет возможности просто запретить. Вот это логично, и правильно. Ваш же вариант, когда программа предлагает юзеру пройти увлекательный квест «угадай, что ты сделал не так» — это какая-то дичь.
Не могу точно вспомнить похожий случай из своей практики, но примерно так:
Есть определенные пользователи вредители, вредители они не вообще, а иногда могут ими становиться, например, в утреннюю смену в воскресенье, куда их задвинули из-за сбоя графика, или из-за того, что «свет с Венеры отразился от верхних слоёв атмосферы и вызвал взрыв болотного газа»
Бороться с таким вредительством организационно очень сложно, тк «других людей у нас нет»
Так вот, эти пользователи могут делать совершенно легитимное действие в системе, например, удаление записи в какой-то таблице. Но прежде чем удалить одну запись, нужно удалить связанные данные в соседних таблицах. Естественно delete cascade выключен. И вот, чтобы что-то удалить из таблицы А, нужно осознанно удалить данные в таблице B. Пользователю в явном виде это не говорится, потому что смотреть выше.
Однажды такой случай дошел до планерки директора предприятия, и мне задавался вопрос, как можно запретить удаление записей такому пользователю. На что я отвечал, что для должности пользователя это легитимное действие, и как система должна отличать, когда можно разрешать действие, а когда нет? Ответа не было.
И да, в системе были и логи, кто что делал, по ним регулярно выявляли очередные «вспышки на солнце». И записи могли не удаляться, а помечаться флагом архив, чтобы не отображались, но в общем и целом человеческий фактор не отменялся.
— Почему они так делают, зачем?
— Потому что могут.
Ну, к примеру, настоящий код ошибки отправляется в лог, который смотрит программист
А вы встречали в продакшне откровенно плохие фрагменты кода, переполненные всяческими проблемами?

Ха, встречали. Мы их и писали!


На вскидку из того, что встречал:


  • Бесчисленные полотна бизнес-логики на SQL.
  • Отсутствие проверок на фатальные значения. Например, round robin, распределяющий товар по магазинам по минимальной партии. Человек упал на клавиатуру, ввел вместо 0,01 0,00001 и подвесил вычислялку (которая на SQL, не забывайте!)
  • Отсутствие проверок на ввод пользователей.
  • Использование в качестве интерфейсов пользователей для ввода данных неподходящих средств (да, я про Excel).
  • Гигантский глобальный Singleton (пытались реализовать Repository, но что-то пошло не так) в монструозном Java-монолите (эта дура потребляла > 300 GB оперативы и расширялась исключительно железом), который все объекты могли изменять, как опкодам вздумается.
  • Ну и чуть-чуть кода (взято здесь, но я-то знаю, откуда он взялся там):
    if ((cBegin_dt.get(Calendar.YEAR) >= cDateLower.get(Calendar.YEAR))
        &&(cBegin_dt.get(Calendar.MONTH) >= cDateLower.get(Calendar.MONTH))
        &&(cBegin_dt.get(Calendar.DAY_OF_MONTH) >= cDateLower.get(Calendar.DAY_OF_MONTH))
        &&(cBegin_dt.get(Calendar.YEAR) <= cDateUpper.get(Calendar.YEAR))
        &&(cBegin_dt.get(Calendar.MONTH) <= cDateUpper.get(Calendar.MONTH))
        &&(cBegin_dt.get(Calendar.DAY_OF_MONTH) <= cDateUpper.get(Calendar.DAY_OF_MONTH))) {
    return 1;
    }

Удачи в разработке, тестировании, деплое и дебаге

Достаточно систем, где на бэке/клиенте лишь самые примитивные проверки ввода и вызовы серверных модулей, но не подлежащих объектов — к ним и обратиться нельзя, права. Отлично себя чувствуют, замечу. Мешок плюсов в придачу, кстати, начиная от повышенной надежности и железобетонной защищенности на уровне sql-сервера, который в этом плане не объехать. Там, где это существенно — этот подход всегда работал. Подход специфический, местами громоздкий, но на своих местах абсолютно уместный.
UPD: из всех перечисленных реальные проблемы тут только со сложностью тестирования, но они примерно точно такие же и с другими подходами. Дебаг тоже, бывает, с приседаниями приходится, но это уже нечасто. Разработка проще и надежнее, деплой как везде.

Многие сейчас стремятся как можно большую часть кода выносить в стейтлесс процессы, а тут не то, что стецтфулл процессы, а сам код стейтом является.

UFO just landed and posted this here
UFO just landed and posted this here
напомнило
if (isPrepared) {
  return true;
else if (!isPrepared) {
  return false;
} else {
  return !true && !false;
}
Я тут прогу учу только неделю, но разве нельзя просто написать
return isPrepared
?
UFO just landed and posted this here

В некоторых языках нужно или лучше написать что-то вроде return (bool) isPrepared, если isPrepared может быть не только булевой.

«Настоящий программист, ложась спать — ставить на столик два стакана. Один с водой. Второй — пустой.»

Как насчёт кода (на плюсах):


Data.p_internalArray[i] = <неважно что>

при том, что класс Data имел operator[] и вообще был фактически цельнотянутой копией вектора, за тем исключением, что всё в нём было public.


Ещё вспомнился один модуль в нынешнем проекте, который сильно-много куда ходит по SSH, но все возможные ошибки конвертируются в один exception на самом верху, типа "не удалось сходить, куда послали, ой". Соответственно логи смотреть бесполезно — только отладчик, только хардкор.

Да что уж там, встречать… мы и сами напишем, вот такую «красоту» например на скорую руку :)
      if (System.currentTimeMillis() > 0) {
        return;
      }
«Во-первых, это красиво» )))
Пришлось сделать так один раз, после звонка «шэф, все пропало, все пропало шэф, наши клиенты получают каждые 5 минут дубли одной и той же смс». Предыстория: по просьбе 1с-ников, мы делали смс-шлюз для 1С наших торговых точек, чтобы те могли рассылать смс простым GET запросом, без вот «этих ваших нанотехнологий». Где-то в районе 30 декабря, когда я весь такой расслабленный, грел пузо на песчаных пляжах, во всех 1С что-то «пошло не так», то ли массовое перепроведение документов приводило к инициации отправки СМС, то ли еще какой-то неизведанный артефакт вылез, история умалчивает.
В сухом остатке: надо срочно вырубить в шлюзе ту часть, которая отправляет запрос на реальный смс-шлюз. Залез на гору, поймал сеть 2G, где-то через час, вспоминая разными словами местного мобильного оператора, я смог войти по ssh на сервер, вставил этот код сразу после кода проверки параметров запроса и до вызова метода отправки запроса на реальный смс-шлюз, перезапустил и пошел продолжать лежать на лежаке и щуриться на солнышко.
Я одно не понял, почему не просто return;?
UFO just landed and posted this here

Просто return не скомпилируется из-за мёртвого кода после него.

Просто return да, не скомпилируется. Но если бы мне надо было сделать такой хотфикс, моей первой мыслью было бы


boolean exitQuickFix = true;
if (exitQuickFix) return;

Скомпилируется, максимум ворнинг от линтера какого-то можно получить, что переменная всегда true. Дергать currentTimeMillis мне бы в голову не пришло, думаю. Разве что на CI настроено любые предупреждения считать ошибками :)

На C# использую конструкцию вида
if (1.Equals(1))

В целях отладки или вырубания кусков кода без ворнингов про то, что выражение всегда истинно и всё такое.
Если джун коллега (ну или и я сам в моменты крайне усталости :) ) по невнимательности
нажмет Unwrap на строке
image

if (exitQuickFix) return; 
то Intellij idea молча удалит весь код, что ниже этой строки. Наверняка это как-то настраивается, но рисковать и закладывать мину замедленного действия, в условиях плохого интернета я не стал.
PS: Кстати, если это конструктор и поля объявленные final инициализируются ниже этой строки с if, то имхо еще и компилятор выдаст ошибку
variable xxxxxxxxx might not have been initialized
Я бы ещё обратил внимание на то, что все password и username написаны в одном стиле — и параметры функции, и переменные, и свойства.
И фиг пойми, password в данном контексте это что именно?
В то же время в SQL не используются данные введенные пользователем и используется строгое сравнение. Джуна избить, но признать что он не безнадежен

P.S. тэг error_message, а там все же тэг а не класс или что еще, тоже вишенка. Пользователь никогда не увидит сообщение. Равно как и LogInFailed что есть то ли длительность показа, то ли опции показа, то undefined
UFO just landed and posted this here
Лет восемь назад я работал в одном маленьком, но очень гордом банке. Вся разработка там велась вокруг веб-ресурсов: пятая пыха, мускуль и адское порождение из тёмных глубин студии Лебедева под названием Parser (даже вспоминать не хочу, это такой образец плохого дизайна, будто консольный бейсик женился на первой, скриптовой ещё, версии пыхи, и у них родился язык-инвалид).
Вскоре после моего прихода уволился один разработчик по имени Руслан, и его грязные делишки свалились на остальных разработчиков.
Этот код был настолько плох, что каждую строчку можно было бы показывать в фильме ужасов: Руслан тварил в каком-то своём стиле, игнорируя любые условности, правила и стандарты. Он забивал на форматирование и отступы скобок, он мешал табуляции и пробелы, заливая ими также и концы строк. Часть файлов он сохранял в win1251, а часть в utf8, а для приведения выводимых кодировок к единому виду писал конструкции с iconv. Он очень любил использовать Codeigniter, но напрочь игнорировал MVC, вписывая html-код форм прямо в контроллер, инлайном или построчно, через echo. А в этот html он инлайном же вписывал и css с js. Он никогда не выносил дублирующийся код в функции, копипастя одинаковые фекальки кода повсюду. При этом он мог написать функцию, вроде такой
private function null2unknown($data) {
    if ($data == "") {
        return "No Value Returned";
    } else {
        return $data;
    }
}

чтобы вызвать её только единожды.

Он, в зависимости от настроения, мог использовать стандартные возможности фреймворка — а мог написать многостраничный велосипед. Он называл поля в своих таблицах «select», «order» и «group» — и да, запрос у него мог вызываться прямо во вьюхе. Он вписывал логины, пароли и абсолютные пути прямо в скрипт, вынося в конфиг только свой почтовый адрес (хотя и его иногда вписывал напрямую). Он никогда не пользовался SVN, но не выбрасывал старый и отладочный код, заботливо сохраняя его в комментариях. Он глушил warning messages собакой.

Он писал чудовищно плохо, глупо и страшно. Я уверен, что Руслан совершал любой программистский грех, который вы сможете вспомнить или придумать. Некоторые руслановы проекты мы, матерясь и плюясь, переписали начисто, некоторые пришлось рефакторить, и три поколения разработчиков кусало губы до крови, занимаясь этим неблагодарным делом. Однажды я даже решил, что худшее позади.

Но как-то потребовалось поменять логику в одном из тех сервисов, что когда-то был Русланом написан, худо-бедно запущен, и с тех пор — неприкасаем. Я открыл код, ещё не зная, кем он был написан — и буквально ощутил, как от этих строк несёт говном. Разбирая это, я чувствовал себя натуральным сантехником, только марал я не руки — я марал в этом мозг.
Итак, вы готовы, дети?
Открывайте спойлер с осторожностью
$sup_time = date("Hi",time());

switch($lang){
	case 'ru':
		$this->setTitle(iconv('windows-1251','utf-8','Информация о заявителе'));
		if ($sup_time=="2356" || $sup_time=="2357" || $sup_time=="2358" || $sup_time=="2359" || $sup_time=="0000" || $sup_time=="0001"){
			$this->load_view('support',$output);
		} else {
			$this->load_view('home',$output);
		}
	break;
	case 'eng':
		$this->setTitle('Payment information');
		if ($sup_time=="2356" || $sup_time=="2357" || $sup_time=="2358" || $sup_time=="2359" || $sup_time=="0000" || $sup_time=="0001"){
			$this->load_view('support',$output);
		} else {
			$this->load_view('home',$output);
		}
	break;
}



С тех пор не было у нас хуже оскорбления, чем сказать коллеге «да ты, друг, написал русланокод».
Итак, вы готовы, дети?
Открывайте спойлер с осторожностью

Ожидал большего, такой код вполне можно быстро и безболезненно поправить. А представьте там были бы не простейшие условия, а "трехэтажные" математические формулы,
описывающие какие-то физические процессы, с сотнями констант вида 531.7087416, которые конечно не помечены как константы средствами языка а вписаны прямо в формулы, и нужно понять что же этот код делает. И все это приправлено обычными toString(boolean_variable) == "true" и перлами типа "variable = variable", которые программиста использовал для подавления предупреждений о неиспользуемых переменных.

Простейшие, да. Но каков подход!

О мой бог. Надеюсь увольняясь оттуда, вы сожгли все это дерьмо за собой

UFO just landed and posted this here
UFO just landed and posted this here
Т.е. вызвали функцию бизнес логики article.put(id,content) и тут же выполнили запрос к таблице и проверили что данные успешно и правильно сохранились.

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

Выделить интерфейс какой-то только для того, чтобы мокать его в тестах норм?

UFO just landed and posted this here

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

Я это знаю, просто поинтересовался. Довольно много людей считает, что выделять интерфейс только для теста — оверхед

Это — «best practices», связанные, например, с такими метриками, как «полнота покрытия кода юнит-тестами». И если вам какую-нибудь такую метрику воткнут в KPI — таки придется ей следовать.
Т.е. вызвали функцию бизнес логики article.put(id,content) и тут же выполнили запрос к таблице и проверили что данные успешно и правильно сохранились.

А можно было бы вместо запроса к базе тут же выполнить article.get(id) и сравнить ответ с эталонным.

Прямо сейчас работаю^W правлю^W живу в проекте, где чья-то светлая голова придумала хранить даты в «минутах с начала эпохи как её видит SQL Server». И по коду везде разбросаны 86400, и всё, буквально всё вообще покрыто этими минутами. А какие тут чудесные вычисления «первого понедельника после события», ооо… А как весело смотреть в базе на дату 63500660…
Самую жесткую жесть, что я видел — это код игры «ЩИ!!! Симулятор жестокости».
вот для затравки кусок:
Одно условие
if(!menu_font||!oboima_text||!info||!infoR||!infoD||!infoBR||
      !oblaka1_tex||!fon1_tex||!fon2_tex||!fon3_tex||
      !galka_tex||!galka_menu_tex||!strelka_menu_tex||!strelka_menu_D_tex||
      !znak_myasnik_tex||!znak_strelok_tex||!znak_razrushitel_tex||!znak_tehnik_tex||
      !status_opit_tex||!status_udar_tex||!status_status_tex||!opit_okno_vibora_tex||
      !okno_lvl_progress_tex||!okno_lvl_progress_red_tex||!okno_lvl_progress_green_tex||!okno_lvl_polzunok_tex||
      !snd||!ak_reload1||!ak_reload2||!pm_fire||!pm_reload1||!pm_reload2||
      !pp19_fire||!pp19_reload1||!pp19_reload2||!fn_f2000_fire||!tt_fire||
      !mac_fire||!mac_reload1||!mac_reload2||!webley_fire||!webley_reload1||!webley_reload2||!milkor_fire||
      !fn_five_seven_fire||!winch_fire||!drob_reload1||!drob_pompa||!vzriv_grena1||
      !rocket_fire||!rocket2_fire||!rocket_polet||!rocket2_polet||
      !myaso_upalo1||!myaso_upalo2||!myaso_upalo3||!myaso_upalo4||!myaso_upalo5||!myaso_upalo6||!myaso_upalo7||
      !myaso_upalo8||
      !myaso_razriv_user1||!myaso_razriv_user2||
      !menu_sound||!menu_choose||
      !shot1||!headshot1||!headshot2||!headshot3||!headshot4||!headshot5||!headshot6||!headshot7||!headshot8||
      !ssik1||!ssik2||!ssik3|!ssik4||
      !ptenec_death1||
      !ak||!ak2||!ak_upgraded||!ak2_upgraded||!w_pm_tex||!w_pp19_vityaz_tex||!w_fn_f2000_tex||!w_fn_f2000_upgraded_tex||
      !w_tt_tex||!w_rpk_tex||!w_mac_tex||
      !w_winchester_tex||!w_winchester_anime_tex||!w_rpk47_tex||!w_glok_tex||!w_glok2_tex||!w_rgd5_tex||!w_milkor_tex||
      !w_panzer_tex||!w_panzer_out_tex||!w_webley_tex||!w_fn_five_seven_tex||!w_granata_podstvol_tex||!w_granata_panzer_tex||
      !w_qlz87_pushka_tex||!w_qlz87_trenoga_tex||!katana_udar_sleva_tex||
      !blood1_tex||!blood2_tex||!blood3_tex||!blood_shot1_tex||
      !blood_plyam1_tex||!blood_plyam2_tex||!blood_plyam3_tex||!blood_luzha1_tex||
      !blood_myaso1_tex||!blood_myaso2_tex||!blood_myaso3_tex||!blood_myaso4_tex||!blood_myaso5_tex||
      !blood_zayac_noga1_tex||!blood_zayac_noga2_tex||!blood_zayac_noga3_tex||!blood_zayac_noga4_tex||
      !blood_zayac_rebra1_tex||!blood_zayac_rebra2_tex||
      !blood_vzriv1_a_tex||!blood_vzriv1_b_tex||!blood_vzriv1_c_tex||!blood_vzriv1_d_tex||!blood_vzriv1_e_tex||
      !blood_vzriv1_e2_tex||
      !player1_myasnik_gogranata_ruka1_tex||!player1_myasnik_gogranata_ruka2_tex||
      !player1_strelok_gogranata_ruka1_tex||!player1_strelok_gogranata_ruka2_tex||
      !player1_razrushitel_gogranata_ruka1_tex||!player1_razrushitel_gogranata_ruka2_tex||    
      !player1_tehnik_gogranata_ruka1_tex||!player1_tehnik_gogranata_ruka2_tex||
      !player1_myasnik_tex||!player1_strelok_tex||!player1_razrushitel_tex||!player1_tehnik_tex||
      !player1_myasnik_ruka1_udar_sleva_tex||!player1_strelok_ruka1_udar_sleva_tex||
      !player1_razrushitel_ruka1_udar_sleva_tex||!player1_tehnik_ruka1_udar_sleva_tex||
      !player1_myasnik_ruka1_pistol_tex||!player1_strelok_ruka1_pistol_tex||
      !player1_razrushitel_ruka1_pistol_tex||!player1_tehnik_ruka1_pistol_tex||
      !player1_myasnik_ruka1_vintovka_tex||!player1_strelok_ruka1_vintovka_tex||
      !player1_razrushitel_ruka1_vintovka_tex||!player1_tehnik_ruka1_vintovka_tex||
      !player1_myasnik_ruka1_winch_tex||!player1_strelok_ruka1_winch_tex||
      !player1_razrushitel_ruka1_winch_tex||!player1_tehnik_ruka1_winch_tex||
      !player2_strelok_tex||!player2_gogranata_ruka1_tex||!player2_gogranata_ruka2_tex||
      !player2_strelok_ruka1_udar_sleva_tex||!player2_strelok_ruka1_vintovka_tex||!player2_strelok_ruka1_pistol_tex||
      !zayac_go_tex||!zayac_uhi_k_tex||!zayac_uhi_s_tex||!zayac_uhi_tex||
      !zayac_boshka_tex||!zayac_boshka_bezuh_tex||
      !volk_go_tex||!volk_trup1_a_tex||!volk_trup1_b_tex||!volk_trup1_c_tex||
      !medved_go_tex||!medved_boshka1_tex||
      !medved_trup1_a_tex||!medved_trup1_b_tex||!medved_trup1_c_tex||!medved_trup1_d_tex||!medved_trup1_e_tex||!medved_trup1_f_tex||
      !medved_trup1_a_bezboshki_tex||!medved_trup1_b_bezboshki_tex||!medved_trup1_c_bezboshki_tex||
      !medved_trup1_d_bezboshki_tex||!medved_trup1_e_bezboshki_tex||!medved_trup1_f_bezboshki_tex||
      !ptenec_go_tex||!ptenec_wait_tex||!ptenec_vpolete_tex||!ptenec_vpolete_reverse_tex||!ptenec_trup1_tex||
      !ptenec_boshka_vzriv1_a_tex||!ptenec_boshka_vzriv1_b_tex||!ptenec_boshka_vzriv1_c_tex||
      !ptenec_boshka_vzriv1_d_tex||!ptenec_boshka_vzriv1_e_tex||
      !RPG_healer_tex||!RPG_illusionist_tex||!RPG_teleporter_tex||
      !bonus_shilo_tex||!bonus_this_tex||
      !bonus_shilo_text_tex||!bonus_this_text_tex||!bonus_daun_text_tex||!bonus_ulitka_text_tex||!bonus_umnik_text_tex||
      !bonus_shilo_status_tex||!bonus_daun_status_tex||!bonus_ulitka_status_tex||!bonus_umnik_status_tex||
      !zayac_trup1_a_tex||!zayac_trup1_b_tex||!zayac_trup1_c_tex||!zayac_trup1_d_tex||
      !zayac_trup1_a_bezuh_tex||!zayac_trup1_b_bezuh_tex||!zayac_trup1_c_bezuh_tex||!zayac_trup1_d_bezuh_tex||
      !zayac_trup1_a_bezboshki_tex||!zayac_trup1_b_bezboshki_tex||!zayac_trup1_c_bezboshki_tex||
      !zayac_trup1_d_bezboshki_tex||
      !zayac_go_bezuh_tex||!ogon1||
      !blood_ssit_tex||!RPG_healing_tex||!vzriv_grena_tex||!vzriv_ogon_grena_tex||
      !alkash1_tex||!derevo1_tex||!penek1_tex||
      !znak_polputi_tex||!polosa_finish_tex)

Так всё правильно — симулятор жестокости к тем, кто решил посмотреть код. Работает идеально.

Господи! Да за такое руки не просто надо отрубать — их нужно очень медленно отпиливать деревянной пилой…
UFO just landed and posted this here

Мясо упало раз. Мясо упало два! Заяц труп!!!

Это уже настолько плохо, что даже хорошо!

for (var i = 0; i < accounts.length; i++) {
    var account = accounts [i];
    if (account.username === username &&
        account.password === password)
    {
      return true;
    }
  }

Не оптимально!
Вот так надежнее:
var authenticated;
for (var i = 0; i < accounts.length; i++) {
    if (accounts[i].username == username &&
        accounts[i].password == password)
    {
      authenticated = true;
    }
  }
return authenticated;

В коде автора цикл завершится сразу, как найдется пользователь. В вашем коде цикл будет проходить все элементы всегда.

Дорогой друг, именно в этом-то и заключается неоптимальность! А вдруг нужный пользователь после нужного, а? Надо проверить их всех, чтобы говорить с уверенностью.

Но ведь authenticated от этого не станет ещё более true, не так ли?

А вдруг компьютер прозевает верного пользователя и не установит true? Тогда, если в списке есть ещё один нужный пользователь, компьютер увидит его и поставит true.


(сарказм)

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

Вы наверное никогда не слышали про paranoid programming? Его еще иногда путают с defensive programming.

Я ещё неопределенное поведение добавил, чтоб не так скучно было.

Но я бы тогда несколько раз цикл прогонял для надёжности, мало ли проскочит. Или лучше промис, который будет в вечном цикле ожидать появление такого пользователя.

Зато с одинаковой сложностью для всех юзеров. Атаки по времени не страшны)

Серьезно? Это как будто спойлер на шаху поставили и сказали, что так стало лучше.

Простите, что куда поставили?
UFO just landed and posted this here
Даже если метод apiService.sql возвращает результат в синхронном режиме (в чём я сомневаюсь), ему нужно открыть подключение к базе данных, выполнить запрос, отправить в точку вызова результат. А эти операции (как несложно догадаться) не могут выполняться синхронно.

Тут имеется в виду «эти операции не стоит выполнять синхронно» или же «эти операции технически не могут выполняться синхронно»? Понятно, если первое, не понятно, если второе. Что им мешает-то так выполняться?

Вот я тоже не понял, почему технически нельзя отправить запрос в базу, получить результат и продолжить...

apiService.sql возвращает результат в синхронном режиме (в чём я сомневаюсь)

XHR-запросы можно делать синхронно, это до сих пор поддерживается в браузерах. Чтобы не возиться с коллбеками или async/await кто-нибудь вполне мог бы такое сделать в реальном коде, почему нет? Удобно же :)


А что что оно блокирует вкладку, ну это может быть и не страшно в его случае — я уверен, что это не худшая UX-проблема того приложения!

Бывает код плохой и страшный. А совсем редко бывает смешной. Вот это тот самый случай.

Вообще, собрать бы коллекцию эдаких шуток на разных языках программирования. Но трудно. Потому что именно смешной код попадается куда реже просто плохого.
А вы встречали в продакшне откровенно плохие фрагменты кода, переполненные всяческими проблемами?
Самое запомнившееся — SQL-запрос в цикле, находящемся внутри рекурсивной функции.

А если в базе дерево хранится в виде узлов и связей?

Это другой вопрос, но там был не тот случай.

Спасибо, не знал этого раньше.

Статья года по повышению самооценки.
А ведь и правда. Можно на собеседованиях давать с вопросом какие тут ошибки. Чем больше найдёшь, тем больше скилл

Больше похоже на методический материал по созданию темы статьи из ничего.

Т.е. никого не смутило, что поиск пользователя делается в цикле, а не средствами SQL? Или я что-то упустил в клиентском SQL?

Зато SQL injection не страшен…

При базе в пару миллионов пользователей — лучше все таки экранировать запрос. SQL injection если захотят, то и так сделают, запрос же с фронта

Какой смысл обсуждать атаку SQL injection, когда в коде есть apiService.sql?

Формально мы не знаем, передаётся ли sql прямо в движок.

На моей прошлой работе был написан телефонный справочник, где личные номера сотрудников скрывались из таблицы джава скриптом. Т.е. клиенту приходила полная таблица, у колонки телефонов стояло свойство display: none, а скрипт после ввода корректного пароля (захардкожен в js) показывал эту колонку. Правда такое было сделано сознательно с полным пониманием рисков безопасности.

Пару недель назад писал парсер для одного сайта с платным доступом к данным, trial аккаунт демонстрировал часть данных, но на самом деле приходят ВСЕ данные в json, и уже скрипт на странице отображает только «разрешенные». Вот до сих пор думаю, есть ли смысл писать разрабам ресурса в надежде на какой-нибудь bug bounty или нет. Там какой-то очередной нишевый бизнес-каталог с профилями компаний, из которых бОльшая часть из 30к профилей содержат только название компании, адрес и сайт (скорее всего обязательные поля при регистрации)

Разрабам — нет. Их бизнесу — есть.

А вот начальник этого программиста должен проконтролировать

автор (да, я вижу, что это перевод — я про оригинального автора) живёт в стране розовых пони с айти-отделами, эджайло-ритуалами, командами и тд. он, кажется, не догадывается, что бывают фриланс-биржи с задачами "надо чтоб зайти на сайт можно было только по логину-паролю и у каждого юзера свой", где каждый мамкин кодер готов за 5 песо запилить CRM.

Или просто начальник программиста не отличает JavaScript от Java

А вы встречали в продакшне откровенно плохие фрагменты кода, переполненные всяческими проблемами?

Как можно? Это же продакшн. Всё вычищено до кристального блеска. От которого иногда кровавые слёзы наворачиваются.

А ещё я уверен в том, что большой процент пользователей применяет одну и ту же пару имя пользователя/пароль в социальных сетях, в сервисах электронной почты, в банковских приложениях и в прочих подобных местах.


На чем основана данная уверенность? Я, например, уверен, что у большинства пользователей в банковских приложениях пароли отличаются от паролей в соцсетях.

Часов люди просто судят по себе

Вспомнил одну из первых игр (на VB, который учил в школе самостоятельно, т.к. в школе был Паскаль). В условиях дефицита знаний, и полного отсутствия книг и Интернета(2002 год примерно) приходилось кодить вслепую. Например линии создавать динамически не умел, рисовать на контролах — тоже, поэтому просто создал на форме массив линий и обращался к ним по имени и индексу, там была возможность объединять их в массив.

Если бы вы заменили линии WinForms на примитивы OpenGL — получилось очень даже быстро :)

В одном весьма популярном плагине файлменеджера к tinyMCE нашел весьма странное решение, связанное с загрузкой картинки и генерацией превьюшки.


Превьюшки генерятся через PHP, самым обычным образом (точный PHP-код опускаю для краткости изложения):


create_image_from_disk()
resize_image()
store_resized_image()

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


Ширину картинки умножает на высоту картинки, умножает на цветность (бит на пиксель, 36 для rgba) и умножает на всякий случай на 2.


И сравнивает это значение с ini_get('memory_limit') (возвращающим значение доступной скрипту памяти в байтах).


Больше — облом-с, отказываемся грузить картинку, сообщаем "слишком большая".


P.S. Обещали исправить в следующей мажорной версии, а пока просто докупите памяти.
P.P.S. следует отметить, что всё таки исправили.

Решение как решение. Или картинка сначала загружается в память, а потом смотрим, хватит ли места?

Если правильно понял, размер картинки оценивается в битах, а объём доступной памяти получается в байтах.

Вы правильно поняли.

Да, это было бы нормальным решением, если бы биты не путали с байтами.

UFO just landed and posted this here

А я как-то делал "SELECT * FROM users", другое дело, что шло оно с фронта через бэк не прямой передачей параметра запроса в sql движок, а через маппинг. Идея была в том, чтобы сделать веб-конструктор отчётов для людей, знающих SQL и виртуальную схему данных.

Не стал читать всё, но показалось, что автор просто расписывает очевидные ошибки в говнокоде. Я надеялся, что тут в чём-то будет скрытый смысл, тайный посыл. А просто расписывать чайниковские ошибки — какой интерес? Никакой изюминки, никакой интриги!

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

Несколько раз пытался написать статью на тему перевода легаси PHP приложения на современные (от symfony 1.2 до Symfony 5 прошло "современные") фреймворки. Каждый раз затыкался на примере легаси приложения. Или подсознательно пишу его так, что статья сводится к "переложите все переменные и функции с префиксом modulename в класс Module". Или пишу такой бред, который бы и в пьяном бреду в 9-м классе не написал.

Много чего встречал, и чего греха таить, кое-что и сам писал в профессиональной юности (правда, всегда парился на тему безопасности). Видел вот такое в продакшене (восстанавливаю структуру кода по памяти, названия переменных условные, не в них суть):


if ($var1 && $var2) {
    // Some code 1
} else {
    if ($var1) {
        // Some code 2
    } else if ($var2) {
        // Some code 3
    } else {
        if (!($var1 && $var2)) {
            // Some code 4
        } else {
            // Some code 5
        }
    }
}

Особенно потрясло, что этот код принадлежал человеку с примерно 15-летним стажем программирования.


Довелось поработать в команде, где тимлид (20+ лет опыта) сам писал и заставлял джуниоров писать (и поддерживать, запрещая рефакторинг) функции с 40+ параметрами. Это выглядит так ужасно, что сразу хочется потерять сознание. Причём в довольно горячих обсуждениях чистоты архитектуры аргументы были такого типа: "Мало ли что писал этот Роберт Мартин, кто он, вообще, такой? У меня этого опыта хоть жопой ешь, я сам себе авторитет.", "Конечно, нормально, чтобы функция была в несколько сотен строк — это достаточно коротко и очень читабельно. И, конечно, у неё всего одна ответственность: делать свою работу.", и т.д. Сильно доставила в то время попытка использовать паттерн Singleton для сохранения состояния между HTTP-реквестами в PHP (никакой особенной конфигурации, классический LAMP). В общем, при наблюдении всего такого иногда трудно отмахиваться от мыслей в духе "человечество обречено"… :)

Попробую принять что этот код серьезный и предположить что есть некоторые значения, которые могут всегда возвращать false при приведении и к true и к false. Не знаю на сколько это актуально для PHP, но в других языках бывает. Тогда положительные значения фильтруются на первом условии, если что-то из них вернуло false смотрится не было ли это первым значением, а может это было вторым, а если оно на все проверки возвращает false — на всякий случай проверяется не являются ли оба значения всегда возвращающими false, а иначе — значит что одно из них не такое и вообще тут можно было бы узнать какое, но, видимо, уже не важно тогда и выполняется Some code 5.

Не говорю что код этот хорош, он очень ужасен. Но, вероятно, я раскрыл его суть :)

Вы сильно польстили этому коду своим вниманием. :) Я видел в продакшене не только этот, но и другой код того же автора. Там нет никакой мистики, просто сочетание методологии "тяп-ляп, и в продакшен" с элементарной невнимательностью (или незнанием) к элементарной булевой алгебре.


Если мы попадаем в первый "else", то это уже значит, что предикат "!($var1 && $var2)" имеет значение "true", нет никакого смысла снова это проверять. Если мы попадаем во второй "else", то обе переменные приводятся к "false", следовательно, предикат "!($var1 && $var2)" опять-таки true, т.е., "if" во втором "else" совершенно лишний. Впрочем, это было понятно уже когда мы зашли в первый "else". В этом коде невозможно попасть в последний "else", т.к. невозможно, чтобы в его "if" было "false" (в этом случае мы бы попали в блок первого "if"). Следовательно, код "// Some code 5" никогда не будет выполнен. При этом, если вообще код "// Some code 5" на самом деле должен быть выполнен, когда неверно, что "!($var1 && $var2)", то он должен быть размещён в блоке первого "if", предикат которого устанавливает именно этот факт. Собственно, в реальной версии этого недоразумения код "// Some code 5" был эквивалентен коду "// Some code 1". :)


Учитывая последнее замечание, полностью логически эквивалентный код, но без жести:


if ($var1 && $var2) {
    // Some code 1 (equivalent to // Some code 5)
} else if ($var1) {
    // Some code 2
} else if ($var2) {
    // Some code 3
} else {
    // Some code 4
}
Скорее всего, исторически был код 2-5, код 1 был перенесен/скопирован позже, а код 5 просто остался

Однако, скорее всего, ошибка в коде всё равно есть (тот код, который "просто остался", надо было перенести в блок 1 — и не факт что это было сделано)

Исходя из

Собственно, в реальной версии этого недоразумения код "// Some code 5" был эквивалентен коду "// Some code 1"


это-то как раз было сделано
есть некоторые значения, которые могут всегда возвращать false при приведении и к true и к false.

Не врубаюсь, что это значит. Можно пример?

ХА! Вы ещё не видели код, который пишут математики!
А что не так с кодом математиков?
UFO just landed and posted this here
Как математик, ответственно заявляю: филологи пишут ещё хуже: Р
Это не просто кусок кода, это сатира на современных JS разработчиков. Увы, чего только не увидишь ныне. И с учётом снижения порога входа и увеличением популярности станет только хуже.

Когда у JS был высокий порог вхождения, начиная с его появления в 1995?

а меня всегда забавляет когда вижу таку простыню тернарных операторов вместо if/else
(var1 ? ((var2 ? (var3 ? var4 : var5) : var6) ? var7 : var8) : var9) || var10

Сначала был один оператор, и все было красиво. Потом дописали условие, потом ещё. Надо бы переписать, но что-то мешает..

Иногда такое вижу изначально, правда в контексте, который исключает использование полноценного JS, только выражения можно.

if ("true" === "true") {
    return false;
}

Могу предположить, что, возможно, раньше условие выглядело как-то вроде


if (account.isNotAdmin === "true") 
{ 
    return false; 
}

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

Почему Вы думаете, что этот код писал джуниор? Даже человек, который в жизни не программировал поймет, что это просто ужасно! if("true"==="true') убил просто

Не нужно делать поспешных выводов. А что если этот код хороший и писал его грамотный программист? Может мы просто не все знаем об окружении. Давай попробуем разобраться.

Начнем с функции authenticateUser:
  var accounts = apiService.sql(
    "SELECT * FROM users"
  );

Это вызов какого-то API. Очевидно, что раз наш талантливый программист делает этот вызов синхронно, этот метод никуда не ходит, а работает с локальным хранилищем. Далее, очевидно, что раз это локальные данные, то в них нет ничего секретного.
АPI не предоставляет возможности отфильтровать данные заранее, значит данных совсем немного. Возможно, это список пользователей, под которыми пользователь уже авторизован.
if (account.username === username &&
        account.password === password)
    {


Все мы, как и автор кода, знаем, что пароли в открытом виде хранить небезопасно. Значит account.password это хеш. А значит переменная password то же содержит хеш. Значит у поля ввода $("#password") переопределен метод val() и возвращает сразу хеш пароля. Интересное решение. Возможно, даже сам JS не имеет доступа к паролю в открытом виде. Точно не через стандартное API. Дополнительная защита, чтобы не выстрелить самому себе в ногу.

  if ("true" === "true") {
    return false;
  }


Исходя из того, что я вижу, я могу сделать вывод о том, что она принимает два аргумента типа string и возвращает единственное значение типа boolean.

На самом деле эта функция возвращает либо boolean, либо undefined.
Как мы увидим далее — код обрабатывает поведение для true (ставит куку), false (выводит сообщение об ошибке) и игнорирует undefined.

  if (authenticated === true) {
    $.cookie('loggedin', 'yes', { expires: 1 });
  } else if (authenticated === false) {
    $("error_message").show(LogInFailed);
  }


То есть это не случайно. Это спроектированная особенность. В каких случаях строка «true» может быть не равна такой же строке? Первое что приходит на ум — это какая-то проверка окружения. Похожие куски странного на первый взгляд кода встречались раньше для определения отдельных браузеров. Но в этом случае логично было бы вынести эту проверку в отдельный метод с соответствующим названием.
Обрабатывается редкая и специфическая ситуация при которой не нужно выводить сообщение об ошибке.
Пожалуй первый серьезный минус кода — отсутствие комментария.

$.cookie('loggedin', 'yes', { expires: 1 });


При успешной проверке пароля мы ставим куку. Что же делает эта кука и весь этот код?
Он берет пароль и логин, и сравнивает их со всеми сохраненными локальными учетными записями. После этого ставит куку на один день. При этом не сохраняет информацию о том, под какой именно учетной записью.
Это выглядит как разблокировка локального хранилища. Но очень простая. Ее можно обойти самостоятельно поставив куку в браузере через консоль. Но это может сделать только тот, кто уже имеет доступ к браузеру с сохраненными куками пользователя. К тому же для этого нужно знать что делаешь. И должен быть доступ к консоли. И консоль.
Значит это не обычный компьютер и не обычный браузер. Возможно это какой-то терминал.
Все равно не очень безопасно, но, возможно, таким было требование бизнеса. Захотелось простую защиту от дурака — кнопку «заблокировать экран» с повторным вводом пароля, но сохранением сессий и автоматической блокировкой через сутки.
UFO just landed and posted this here
У меня два варианта почему так:
1. SQL интерфейс поддерживает WHERE, но требует соответствующих интерфейсов от хранилища, на которое мапится конкретная таблица. Т.к. users очень маленькое хранилище, разработчик решил пока не тратить время и не добавлять поддержку WHERE.
2. WHERE для users реализован, вот только поля password и username «магические». Их динамически формирует соответсвующий класс JS и выборку по ним в рамках запроса делать нельзя
UFO just landed and posted this here
если users весьма хитрое представление и прав пользователю на сервере дано в обрез, то это просто шутка над читателями.
Может быть я неосознанно познал дзен, но код из статьи не вызвал какого то лютого батхерта, вот если бы автор этого отстаивал Идеальность этого кода или нынешнее руководство хвасталось и ставило автора мне в пример, тогда может быть да, полыхнуло, а так просто забавно)
«отсутствие у того, кто её писал, некоторых базовых знаний о программировании»

Простите, автор, статья интересная, но здесь вы ошиблись, т.к. демонстрируется вовсе не отсутствие базовых знаний — с ними всё ОК у пациента разбора. Демонстрируется пробел в знании работы веб архитектуры. А это ОЧЕНЬ разные знания, пусть и приходится их всем в той или иной мере знать (часто вынужденно, как мне, сисадмину)
Коллега (я — тоже сисадмин), а вы не могли бы пояснить подробнее, какие именно особенности работы веб-архитектуры не позволили в рассматриваемом фрагменте кода написать в конце функции просто return false, а заставили автора кода оборачивать этот оператор в конструкцию if («true» === «true») {… }?
Судя по всему, это такой «юмор» у погромистов. Вы ж понимаете, коллега, что %% (надеюсь они не видят этот комментарий) шизофреников и аутистов в их среде намного выше, чем в нашей сисадминской среде или даже в армии(!), или в каком-нибудь провинциальном деревенском похоронном бюро… В то же время и %% гениев среди них достаточно велик. В общем, юмор у них такой — моё предположение.
Что-то вроде
while 5>1:
    do something forever
%% (надеюсь они не видят этот комментарий) шизофреников и аутистов в их среде намного выше,

Мы тут обсуждаем качество кода, а не того, кто его написал, не так ли?
И я правильно вас понял, что разумных причин написать так, как написано, вы не видите?
Тогда причем тут веб-архитектура? Это просто неграмотно или неряшливо написанный код, согласны?
Нет, не согласен. Это не неряшливость — однозначно. Если вы работаете с разработчиками решений, то знаете, как выглядит именно неряшливый и поганый код, особенно в контексте «ОТСУТСТВИЕ БАЗОВЫХ ЗНАНИЙ В ПРОГРАММИРОВАНИИ».
Читайте цитату в кавычках по слогам, а не по диагонали. Должно помочь.
Здесь именно юмор.
И таки нет, мы обсуждаем не только код, но и того кто его написал.
Веб-архитектура при том, что котлеты и мухи это разные сущности. Код это код, а веб это веб. Поймите, коллега, ЭТО РАЗНЫЕ СУЩНОСТИ. Это как воздух и вода. Мужчина и женщина. Как я и вы.

Веб работает без кода?

Если вы работаете с разработчиками решений, то знаете, как выглядит именно неряшливый и поганый код

Ну, вообще-то я долго работал имено программистом, а в админы ушел сильно после 35 лет. Так что я отлично понимаю, откуда такой код берется (сам такой писал, если чо), и полностью согласен с DrPass ниже, как именно, скорее всего, возник данный конкретный перл: там была убрана проверка условия, но — не насовсем, и на всякий случай оператор if был оставлен.
Суть моего комментария в другом: к особенностям «веб-архитектуры» этот странный кусок кода никакого отношения не имеет. Писать код надо без таких вот странностей для любой архитектуры. Не всегда это по жизни получается, правда, но это — другой вопрос.
Я не сисадмин, я программист, поэтому ваша точка зрения мне не очень понятна. С точки зрения программиста, там вообще два варианта:
1. Это вообще не веб-архитектура (по крайней мере, фронт приложения остался за кадром), а весьма корявый серверный код, на уровне среднего джуна, который что-то знает, что-то нет. Например, не знает, что в SQL есть конструкция WHERE, что вытянуть весь датасет и перебрать, это маздай, и что пароли принято хешировать, и так далее.
2. Это всё-таки фронт веб-приложения, и тогда это вообще лютейшая дичь со дна разработки, в которой фронт с базой работает через сервис, выполняющий прямые SQL-запросы. И это отсутствие ключевых базовых знаний.
Поясню: понятие «джун» весьма размытое в нашем мире, но раз уж приходится иногда их гонять, особенно когда они лезут туда куда не нужно в процессе разработки )), то я имею небольшое право УТВЕРЖДАТЬ, что «средний джун» это уже никак не «отсутствие базовых знаний о программировании» — я писал именно об этой неувязке. То что чудить могут и лидеры проекта вместе с подчинёнными тоже факт и это ответ к п.2 — это может быть фронт какого-то глубоко приватного «приложения», а там чего только не лепят в качестве костылей и заплаток — работает и ладно. Представленное решение может быть настолько временным, что мы здесь букв и нейронов больше сжигаем над обсуждением этого, чем оно в реальности стоило автору кода (там на 95% прям со скриптами обработки валяется какая-нибудь sqlite.db в которой и лежит «датасет» из трёх строк).
Конечно же «в мир» такие решения уходить не должны. Убеждён, что это временное изделие для внутреннего (временного) пользования
небольшое право УТВЕРЖДАТЬ, что «средний джун» это уже никак не «отсутствие базовых знаний о программировании

Отсутствие базовых знаний о программировании — это ведь широкое понятие. Это не только «полное отсутствие каких-либо знаний о программировании», но и «существенные пробелы в определённых важных областях». Человек какие-то базовые вещи знает, какие-то не знает, и соответственно имеет недостаточно квалификации, чтобы вот так выдавать в прод без ревью со стороны кого-то более опытного.
это может быть фронт какого-то глубоко приватного «приложения», а там чего только не лепят в качестве костылей и заплаток — работает и ладно.

Я могу сказать так, что раз в этом приложении как минимум есть потребность в аутентификации с логином/паролем, значит, оно уже не настолько глубоко приватное, чтобы делать его настолько дырявым. Тем более что привычная аутентификация без существенных косяков в реализации ничуть не сложнее, чем вот этот франкенштейн, и единственная логичная причина, почему её не сделать — это потому что разработчик не в курсе, как надо.
Всё что вы пишете верно, но есть одно «но», как высказался выше мой нудный серьёзный коллега:
какие именно особенности работы веб-архитектуры не позволили в рассматриваемом фрагменте кода написать в конце функции просто return false, а заставили автора кода оборачивать этот оператор в конструкцию if («true» === «true») {… }?

И вот с этого момента начинается «но» — здесь явно не незнание основ программирования, а НАМЕРЕННАЯ провокация. Можно признать её попыткой пошутить, съюморить, сделать специальный кейс для разбора красноглазыми очень серьёзными критикам. Keep calm, brothers. Be easy
Меня этот кусок кода вообще не смутил, т.к. я не раз видел подобные вещи, и происхождение у них вполне однотипное: у чувака там раньше было написано что-то вроде if (someVariable === «true»), потом в процессе модификации функции someVariable отмерла, а поскольку делалось на коленке, весь блок if автору было или лень вычищать, или решил оставить на всякий случай, вдруг придётся переменную вернуть, вот он и просто меняет someVariable на «true»
Sign up to leave a comment.