Как стать автором
Обновить

Комментарии 165

m_bTypeED = ( nCmd == nCmd )? TRUE: FALSE; В C++ вполне себе жизнеспособная штука, единственный способ проверить на Undef т.к. Undef не равен ничему по определению. Доказательство codepad.org/HJVKFaai
т.е. если в переменной undef то сама себе она не равна.
Классическая проверка:
double x = ...;
bool isNan = x != x;
Извените, я на С++ не пишу, поэтому может задам глупый вопрос: а разве просто вот такого «m_bTypeED = ( nCmd == nCmd );» не достаточно?
TRUE и FALSE в данном случае константы, и что в них определено нам неизвестно.
Нет, во-первых получим ворнинг если эти макросы определены числами, во-вторых — неизвестно какие это числа и как их могут переопределить впоследствии (всякое бывает).
Warning будет, т.к. typeof( BOOL ) != typeof( bool )
Ну про такие случаи и я и PVS-Studio знает. :) Это переменная integer типа. Я просто в примерах расписывать не стал типы и так далее. А то скучней будет код рассматривать.
PVS-Studio. PVS-Studio. PVS-Studio. PVS-Studio. PVS-Studio. PVS-Studio.
Можно хоть в комментах не рекламировать?
А где реклама вне комментария? :)
Тут, хотя бы. PVS-Studio умеет то-то, а еще вот то-то а еще! А еще!11 А еще блины печь, вот! Вы, ребята за это деньги берете — и так рекламируете, постыдились бы что-ли, не маленькие ведь уже.
Спасибо за поддержу, за рекламу и за ссылку.
Да, мне прямо Шахиджаняна чем-то напоминает, который любит между строк ввернуть свое соло :)
Ясное дело, что nCmd не float.
«Ясное дело» — большинство багов именно так и появляются.
Он хорош ещё и тем, что хоть TRUE и FALSE определены как 1 и 0 соответственно, не значит, что нужно неявно преобразовывать true в TRUE, а false в FALSE.
НЛО прилетело и опубликовало эту надпись здесь
Undefined — неопределённое значение, NaN — Not a number — не число. Если поделить 0 на 0, ил сделать что-нибудь подобное, получится нечто особенное, именно это и обозначили такими именами. Сходите по примеру что я привёл и посмотрите как это работает в коде.
NaN это вполне определенное значение, я не согласен с данной терминологией.
NaN да определённое, но всё равно хитрое и особняком стоящее. Правильные формулировки стоит читать в соответствующих стандартах. А то что я написал это просто краткое неточное пояснение на пальцах.
а сори, понял про что, да косяк. давно на С/С++ не кодил
NaN — это не определённое значение. Два NaN могут быть сильно разными NaN. Например 1/0 = NaN, sqrt(-1) = NaN (в действительных числах), 1 + NaN = NaN (и нельзя сказать что это одно и то же, единицу то мы прибавили всё-таки).
1/0 = +inf
Кто сказал?
NaN — это определенное специальное значение. Специальное — значит, что операции с ним происходят по определенным правилам, отличающимся от операций с обычными числами. Определенное — значит, что определены операции которые его порождают и определены операции с другими значениями, в т.ч. с самим собой. NaN полученный из 0/0 и из 1 + NaN ведут себя во всех операциях одинаково, следовательно ничем друг от друга не отличаются, следовательно сказать, что они разные нельзя.
Все значения ведут себя определённым образом. Даже undef, null и что бы то ни было ещё. Но по определению NaN != NaN. Так сказал IEEE и это вполне логично.
некоторые операции (вообще) могут пораждать Undefined behavior, вот что я называю неопределенностью. C NaNом же все понятно.
НЛО прилетело и опубликовало эту надпись здесь
Что такое undef? NaN наверное?
Да заметил, косяк в моей терминологии.
Цикл для постижения Дао доставляет.
А что такое…
Если *tbl предваритено занулить, то получится некое количество…
*tbl==num_tbl
«для надёжности пересчитали» :)
На самом деле мне кажется что там теоритически может быть и не так просто. Например если tbl — указатель на некий инстанс класса, у которого переопределен оператор "++" таким образом, чтоб он выполнял какое-то осмысленное действие.

К примеру класс «PeopleInRoom», который при "++" добавляет новый инстанс класса Person к своему списку людей в комнате.

Да, я тоже всецело за читабельный код и считаю это плохим стилем (так, как не однозначно читается), но такое вполне возможно.

ПС. Если я ошибаюсь, и в С++ такое невозможно — поправьте
Ну следуя такому стилю следовало бы перегрузить оператор +=, так что в любом случае говнокод :)
Внутри был бы тот же цикл, а если в проекте чаще используется "++" без цикла, и цикл нужен только в одном методе, то зачем переопределять "+="?

Как бы все зависит от остального кода, но я лично в случае, если это действительно указатель на инстанс класса с переопределенным оператором, не стал бы называть это «говнокодом». Максимум — не оптимальным.

Но если, как ниже уже автор отписал, это int, то, безусловно и говнокод.

Так что это скорее хороший пример того, как нельзя давать код снипеты без более полного описания то, с чем работает снипет.
Если к большинству говнокода ещё и историю изменений приложить, то оказывается что это не говнокод, а просто незамеченные хвосты :)
flag = true;
flag = false;

Обкатка переменных…
скорее всего осталось после отладки, не судите строго.
У меня раньше бывало, когда ещё не было спец. функций своих, что после отладки я искал вывод значений переменных минут пять-десять
Прогрев проводов :)
Намекаете на теплый ламповый код?
Прогрев процессора перед началом серьезных вычислений. :)
видимо комментили в режиме отладки то одну строку, то другую.
в итоге ничего не пригодилось
Такое часто случается, когда проект в своем жизненом цикле меняет много разработчиков. Первый забывает удалить неиспользуемые переменные(или что-то в этом роде), второй меняет значение переменной, но не только «боится» удалить прежнее значение, но и не комментирует его(первоначально не понимая для чего она), чтобы не попасть случайно в неприятности с первым. После некоторого времени приходит третий, добавляет, реже удаляет код и переменная(и не только) остается «unused».
Напомнило старую цитату с баша:
<******> к вопросу о вчерашних скриптостраданиях. Только что кодер знакомый прислал, нашёл в коде программы, написанной уволенным коллегой незадолго до ухода:
<******> #define TRUE FALSE //счастливой отладки суки
* ****** такого извращённого юмора ещё не встречал
#define true (Math.random()>0.5)

Еще веселее.
А вот это уже боевой прием:
#define private public
#include <****.h>
Pimpl на этот прием смотрит…
Это не просто боевой приём, а [анти]паттерн Павлик Морозов.
image
void get_tomorrow_date( struct timeval *date )
{
    sleep( 86400 ); // 60 * 60 * 24
    gettimeofday( date, 0 );
}
Большая ошибка! Этот код возвращает завтрашнюю дату завтра. Чтобы получить завтрашнюю дату сегодня, надо добавить sleep( -86400 ) в конце.
Названия приводить не буду, так как считаю это неэтичным.

Успешно нашел источники 7/10 через Google (и code search).
ну уж напишите в комментах для любопытных.
А вы посылаете патчи? Т.е. скажем так способствуете уменьшению энтропии? :)
Да.
Э, батенька, Вы обфускацию вредоносов ещё не видели! :)
НЛО прилетело и опубликовало эту надпись здесь
image
Только из-за картинки и прочитал пост!
В коллекцию однозначно!!!
image
«Цикл для постижения Дао» основательно вколотил в пол, даже больше камент чем код :) Это замечательно.
Отличные примеры :)
Хотя у всех наверное были куски кода, за которые стыдно, глядя на них, спрашиваешь себя: «как я это написал?»
Отправляйте материал на govnokod.ru и можно смело удалять топик.
О, сегодня вспоминал как он называется.
Отправляйте gnomeby на «одноклассники» и можно смело удалять аккаунт.
Как надоели со своим «не надо Ctrl-V»!!! Один сплошной выпендреж. Это уже стало на религию походить.

При переписывании рабочего участка заново наделать ошибок значительно больше шансов, чем не все поправить в скопированном. Надо искать возможность контроля скопированных участков по средством IDE а не зубрежки.
По-моему там проще циклом было пройтись или я не прав?
Цикл — это да. Я против догмы о вредности copy&paste. Не внимательный программист ошибку допустит в любом случае. А внимательный сэкономит время.
Ну это бесспорно :) Не было бы копи-пейста не было бы половины игр/фильмов/программ и т. д. и т. п. :)
Вы говорите так, как будто бы «невнимательный программист» и «внимательный» — разные породы людей.
Отвлекли человека вопросом — и вот он уже «не внимательный».
Совершенно верно.
Поэтому проводить всякие рефакторинги настоятельно рекомендую в одиночку (по вечерам или на выходных)
Предлагаете работать в нерабочее время?
А в рабочее время чем тогда заниматься? :)
>>Не внимательный программист ошибку допустит в любом случае
[sarcasm] ну вы понЕли [/sarcasm]
а можно и memset, но бегать по рабочему коду и править такие мелочи — прямой путь к настоящим ошибкам.
Цикл — дольше в исполнении.
А вот не факт.
Код небольшого цикла для инициализации пусть даже какого-нибудь петабайтного массива весит несколько байт, и весь поместится в дорогущей но ооочень быстрой памяти в кеше процессора. А линейный код — постепенно будет из памяти вычитываться. Так что было бы интересно просчитать, при каких параметрах быстрее цикл, а когда — линейный.
А еще вспомним оптимизации вездесущих компиляторов…
А вот для меня верно как раз обратное.
Если я копирую строчку кода, а потом заменяю, например, x на y в нескольких местах, то гораздо выше вероятность, что где-то забуду заменить, чем если с нуля всю строчку написать.
Пусть заново писать и медленнее, но баг все равно дольше исправлять.
Лучше всего выносить нужный участок кода в функцию, после чего ее заиспользовать.
В большинстве случаев добавив пару параметров функции это удается сделать.
Естественно, эти операции проводим не ручками, а средствами IDE, чтобы избежать потенциальных ошибок copy&paste.
дело не столько во внимательности при исправлении. дело в том, что схожих по функциональности частей кода быть не должно.
Когда б вы знали, из какого сора
Растут стихи, не ведая стыда

/А. Ахматова/
Мне по определению стыдно за код который написал до сегодняшнего дня. И так каждый день…
Этому причина профессиональное самосовершенствование.
Этому следствие профессиональное самосовершенствование. :)
И это хорошо.
Только при чём тут C++? Такое на чём угодно написать можно.
Подсказка: PVS-Studio работает с С++-кодом ;)
НЛО прилетело и опубликовало эту надпись здесь
При беглом просмотре не заметил ничего, что нельзя было бы написать на любом (наверное) Си-подобном языке, ничего специфического для C/C++ типа прямой работы с памятью не заметил.
На Java пример 1 не сработает, т.к. операция || определена только для boolean-переменных. К примеру.
1. А что не так в примере 3?
2. По-моему, в примере 2 условие как раз всегда ложно.
*tbl += num_tbl; — вот и весь третий.
Неее! Там еще запутанней. Тот код, что приведен, просто увеличивает указатель:
tbl += num_tbl;
Звездочка там ничего не значит. Лишняя.
Есть подозрение, что на самом деле там должно быть написано в духе:
for (i = 0; i < num_tbl; i++) {
*tbl++ = X;
}
По исходному коду непонятно, указатель ли это, или объект с перегружеными операторами инкремента и разыменовывания. Поэтому у меня лично ни один волос дыбом не встал. Вот если бы было нечто такое:

for(i = 0; i < num_tbl; i++)
num_tbl++;

то это уже был бы настоящий дзен :) Хотя такую ошибку быстро бы отловили ввиду явного зависяния программы.
Ну зачем мне в заблуждение читателей вводить. Если бы это был хитрый код с классами, я бы этот пример не приводил. Это обыкновенный указатель на unsigned.
об этом как-то упоминуть стоило все-таки.

Судя по комментам не я один подумал о перегруженном ++ в классе (мой пример когда это вполне может быть выше).
Код в 3м примере вполне имел бы смысл, если бы tbl был volatile.
Что плохого в 4?
Для 4 придумали циклы.
Не совсем. Один вызов printf будет эффективнее 9-ти вызовов. Даже если компилятор развернёт цикл. Ниже правильно написали, запись m[0, 2] в C++ эквивалентна m[2] — смотрите оператор ",", т.е. будут выведены указатели как вещественные числа. Надо писать m[0][2]. А через запятую пишется в Pascal. Отсылка к BASIC менее вероятна — там вообще круглые скобки для массивов используются. Да и студенты чаще Pascal учат.
Развертывание циклов компилятор как правило сделает эффективнее, чем Вы ручками.
Не думаю, что он объединит вызовы функций в один. Да, в примере 8 — однозначно цикл делать, но в данном случае особой пользы от цикла нет, если это, к примеру матрица 2D преобразований — её размерность не изменится, а один вызов или 9 может влиять. Хотя тут, скорее всего отладочный вывод оптимальность там не нужна. Но в общем-то ошибка не в этом.
цикл возможно и развернет. А вот строку формата он не составит.
m[0, 0] <-> m[0][0]
На самом деле все довольно тривиально, открывая топик я надеялся увидеть что-то действительно хитрое:)
О, я в старом рабочем коде видел вообще шедевр (по памяти):
int get() {
   int something;
   if (isAction1()) 
      return 1;
   if (isAction2())
      return 2;
   return something;
}


Т.е. если другие условия не прокатили, верни хоть что-нибудь :)
Ха. А как вам такое?

bool getSmthValue() {
  bool someParam = someExpression();
  if (someParam == TRUE)
   return TRUE;
  else if (someParam == FALSE)
   return FALSE;
  else
   return (!TRUE && !FALSE);
}
тут неизвестно что такое TRUE и FALSE может там есть еще какие-то константы типа DONT_KNOW
тогда конец должен был бы быть return DONT_KNOW;
Вспоминается статейка про C+- (более-менее Си)
тогда уж FILE_NOT_FOUND %)
Не, ну тут хоть константы. А там ведь реально можно было получить неинициализированный something…
(!TRUE && !FALSE) == FALSE
Парадокс :)
А вы про оператор po_lyubomu не слыхали?
if (is_this()) { return 1; }
if (maybe_this()) { return 2; }
po_lyubomu { return something; }
ну вот, а вы все «пых», да «похапе». Этот топик — очередное доказательство того что говнокод возможен везде, и качество говнокода напрямую зависит от его автора)
Просто некоторые языки не позволяют совершать глупых ошибок и сильно ругаются. В PHP даже синтаксически неверный код может иногда работать. Или не показывать, что он не работает ;) Но и на нём можно писать качественный код, было бы желание.
В PHP даже синтаксически неверный код может иногда работать. Или не показывать, что он не работает ;)
Не могу не согласиться)
То ли дело перл — берешь прогу, которая работает, стабильная, не глючит. Смотришь код. И видишь, что эта мешанина ASCII символов она не то что правила синтаксиса нарушает, она вообще богопротивна!

(тут я хотел вставить для примера какой-нибудь нечитаемый перл-код, погуглил, и уже нашел какую-то бредятину, которая, видимо, что-то должна делать осмысленное, начал читать вокруг что же это они такое страшное и непонятное запрограммировали на перле, и выяснил, что это на javascript).
Перл, кстати, очень жесткий язык. Да, код бывает запутанный и неочевидный (почти всегда, хе-хе :), но он построен по всем правилам языка, а иначе он тупо не будет работать. Но из-за того что код часто не очевиден он заставляет разбирать его логически практически каждый раз, что ни в коем случае не вредит качеству кода.
Да говнокода как такового в примерах не присутствует, разве что пункт 5.
зато на «пыхе» он получается намного проще и веселее изза более фривольного стиля — по себе знаю :)
Не только от автора, еще и от окружения.
Еще хочу заметить. Компиляторы C++ известны своими оптимизторами. Иногда в попытке разобраться «кто дурак» такие метамарфозы пишутся… И на радостях от того, что разобрались, метамарфозы врастают в релиз. А потом тебя, программиста с опытом, образованием, и что не маловажно, положительным экономическим эффектом величают быдлокодером.
Плюс, примеры выдернуты из контекста.

Как известно бывает и так, что нормальный, на первый взгляд код, при анализе оказывается крайне фейерическим говнокодом)
Я бы не сбрасывал «Цикл для постижения Дао» так быстро со счетов — он может иметь смысл если tbl это custom-made итератор для которого operator * имеет какой-то нужный побочный эффект.
Чаще такие «нужные» побочные эффекты прячут в оператор ++, а не *. Я бы «быстро сбрасывал со счетов» авторов таких эффектов.

Поддерживали мы как-то код, написанный в Австралии, кажись. И вот коллега спрашивает — «как ты думаешь, что делает этот оператор?». А я уже знаю, что код говно, по диаметру широко открытых глаз коллеги понял. И думаю — ну какое самое вонючее дерьмо там может быть? И наобум говорю: «там рисование». Угадал. У коллеги был двойной шок. Первый от кода, второй от глубины моего дао :))
Нет. Там простые типы. Я загромождать не стал.
Есть такой замечательный проект. The daily WTF :) Погуглите. Там этого добра :))))

И не бередите рану, а то я ещё начну Джавовские перлы выкладывать. Знаете, тоже накопилось воз и тележка ;)
7 — прекрасен. Я тоже встречал эту ошибку в рабочем коде — когда вместо clear() используется empty() которая не чистит, а лишь проверяет контейнер =)
Это проблема скорее именования, чем использования. Да, стоило поглядеть документацию к классу, но иногда делаешь что-то по аналогии.
Создатель класса обозвал лучше бы типа isEmpty или что-то подобное.
По выдернутому куску кода неясно, каким классом реализуется fspecs. Может быть метод empty там и правда что-то чистит. Хотя похожие подозрения возникли и у меня, но бездоказательно :) Имя метода empty можно покритиковать в любом случае, за неоднозначность.

Мне там больше понравился единственный return, выход всегда с OP_SUCCESS. Достаточно часто встречал столь безудержный оптимизм.

Ну и комментарий мне там совершенно не нравится, несмотря на длину и подробности. Он объясняет, как «вышестоящие» классы должны использовать этот NEPContext. Имхо, таким комментариям место именно в тех «вышестоящих» классах. А тут надо было лишь объяснить внутреннее устройство NEPContext, не внешнее его применение непонятно где, с пояснениями логики работы непонятно чего.
А еще можно добавить забытые assert(), которые не компилируются в дебаге и мое любимое
if ( a == b )
return true;
else
return false;
Если вы про return (a == b); то лучше смотрите сюда:
int a = 0;
if (true) { a++; }
if (1 == a) { return a; }
Хех :)

if (strlen(boolToString(var)) == 4) {...}
Так удобнее брейкпоинты на строчках расставлять! ;)
Как жаль, что тот код, с которым мне приходится иметь дело, не открытый и я не могу его вот так вот выложить. А то периодически наступают психологические травмы от погружения в кастомерский код :)
А вы имена переменных поменяйте и выложите:).
Полагаю вы промахнулись с ответом на мой пост. Слишком там всё серьёзно чтобы рисковать. К тому же даже с другими переменными код будет узнаваемым.

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

Отдельную категорию занимаю программы возрастом 20-30 лет написаные на макросах и после препроцессинга превращающиеся с сущий ад. Остаётся только выть глядя на результат.
m_bTypeED = ( nCmd == nCmd )? TRUE: FALSE;


Это макетка для дальнейшего расширения, когда код постоянно дополняется, или будет меняться алгоритм.

Например, я часто не комментриую и не убираю условие, а просто добавляю 1|| или 0&&:

if (1||очень_большое_условие) {

}


Это для того, чтобы потом быстро опять сделать, чтобы условие заработало.
Скорее всего во втором примере "!" и "-1" компенсируют друг друга)
Пример 2:
! (fp = fopen(filename, "wb")) == -1

У  ! приоритет выше, чем у ==, отрицание чего угодно, как я понимаю, должно быть либо 0, либо 1, поэтому условие все-таки всегда ложно, а не всегда истинно.
Да. Поправил. Значение типа bool всегда неравно -1.
Предполагаю что условие должно выглядеть как

if ((fp = fopen()) == NULL) {
// foo bar
}
Так и есть.
Но вообще я против и такого кода. Вот ведь жадины. Строчку сэкономили. По мне, намного лучше так:
fp = fopen();
if (if == NULL) {
// err
}
if (mysql_query('DROP TABLE таблица'))
echo "таблица существовала";
4. Первородный грех использования Basic.

Не грешите на старика
10 FOR I=0 TO 2
20 FOR J=0 TO 2
30 PRINT M(I,J)+" "<strong>;</strong>
40 NEXT J
50 PRINT
60 NEXT I

Код не совсем идентичен, не хотелось гуглить детство, но принцип, наверное, понятен
На govnokod.ru еще и не такие забавы можно увидеть))
Удобные размеры массивов:

#define KB * 1024
#define MB KB KB
#define GB MB KB
#define TB GB KB

int main( ... ) {
char * arr = new char [16 MB];
...
}
по-моему — ничего так мысля.
Да, но на практике это записывают обычно более надежным способом:

enum {
  KB = 1024,
  MB = 1024*KB,
  GB = 1024*MB
};
...
char* arr = new char[16*MB];
Согласен, что так лучше, но что-то в дефайнах таки есть.
Грабли там есть
Ничего хорошего в дефайнах нет вообще:)
большая часть примеров могла быть следствием недорефакторинга
Ох спасибо, повеселили…
Вообще, я подобного кода на питоне насмотрелся. Например, копирование словаря циклом по iteritems, вместо того, чтобы просто сделать copy() — и это было написано вполне себе неплохим программистом, преподающим сейчас в бауманке…
В общем-то, aspect прав, это действительно может быть следствием поспешного рефакторинга.
Да зря вы смеетесь над циклом для достижения Дао, я почти так делаю (я еще добавляю присвоение в некую переменную через xor, который возвращается из функции, чтобы компилятор не убрал код совсем) для прогрева данных после мапирования их из диска, т.к. таких областей я мапирую много, а последовательное чтение по диску лучше чем хаотичное, когда все эти области начнут считываться.
Тот же код может применяться для прогрева кеша определенной областью данных из памяти.
Тот код ничего не читает и не пишет. Он только увеличивает указатель. Разыменование указателя происходит уже после.
Пример 2 конечно же недопеределка, видимо, был код if ((fd=open(...)) == -1) который при переделке к fopen забыли убрать == -1

Вероятно именно так.: Осталось ещё разгадать, откуда взялось отрицание '!'. :)
элементарно, NULL проверяет
А почему кстати до сих пор нет облачного сервиса для код ревью? Например я написал свой первый код на Haskell и хочу чтобы меня потыкали знатоки, за $ например. Нет такого?
Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Публикации

Истории