Не вижу в коде НИЧЕГО криминального. Я понятия не имею, для какого компилятора и среды это написано, и знаю, что есть много примеров, когда использование goto и подобного способа хранить данные в стеке вполне оправдано.

Здравствуйте, slavo, Вы писали:

S>увольняю

Что, человек реально невменяем?
Это был единственный косяк, или для него это характерно?

Здравствуйте, slavo, Вы писали:

S>Что делать с сотрудником, который написал такой код?
S>

S>if((int)ici_os.a_top[-1] == 0xfdfdfdfd || *(int*)ici_os.a_top[-1] == 0xcccccccc))
S>{
S>  ici_error = "stack error";
S>  goto fail;
S>}
S>

А он случайно не из Индии родом?

Здравствуйте, Corvin, Вы писали:

C>Здравствуйте, slavo, Вы писали:

S>>Что делать с сотрудником, который написал такой код?
S>>

S>>if((int)ici_os.a_top[-1] == 0xfdfdfdfd || *(int*)ici_os.a_top[-1] == 0xcccccccc))
S>>{
S>>  ici_error = "stack error";
S>>  goto fail;
S>>}
S>>

C>А он случайно не из Индии родом?

Помнится, у нас на проекте один из директоров конторы-заказчика как-то написал такую обертку:

void* UMW_Alloc(int nSize)
{
   if (nSize > MAX_MEMORY)
      return malloc(MAX_MEMORY);
   else
      return malloc(nSize);
}

Тот, кто первый на это наткнулся, долго искал причину.

Здравствуйте, slavo, Вы писали:

S>Что делать с сотрудником, который написал такой код?
S>

S>if((int)ici_os.a_top[-1] == 0xfdfdfdfd || *(int*)ici_os.a_top[-1] == 0xcccccccc))
S>{
S>  ici_error = "stack error";
S>  goto fail;
S>}
S>

Если это отлов какой-то бяки, то это ООООЧЕНЬ аккуратный товарищ. Я бы написал что нидь типа
S>

S>if((int)ici_os.a_top[-1] == 0xfdfdfdfd || *(int*)ici_os.a_top[-1] == 0xcccccccc))
S>{
S> int iii = 0;
S>}
S>

+ бряк.

Здравствуйте, slavo, Вы писали:

S>Что делать с сотрудником, который написал такой код?
S>

S>if((int)ici_os.a_top[-1] == 0xfdfdfdfd || *(int*)ici_os.a_top[-1] == 0xcccccccc))
S>{
S>  ici_error = "stack error";
S>  goto fail;
S>}
S>

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

Здравствуйте, StevenIvanov, Вы писали:

SI>Антипаттерны:
SI>- магические числа

Эти значения программистами узнаются лучше, чем какие-то имена для них.

SI>- платформозависимость
SI>- использование goto
SI>- (если код С++) — использование C-style cast

Сами такие антипаттерны придумали или подсказал кто?

Здравствуйте, slavo, Вы писали:

S>Что делать с сотрудником, который написал такой код?
S>

S>if((int)ici_os.a_top[-1] == 0xfdfdfdfd || *(int*)ici_os.a_top[-1] == 0xcccccccc))
S>{
S>  ici_error = "stack error";
S>  goto fail;
S>}
S>

У меня создается впечатление, что Вы просто не знаете что это за константы и от этого начинаете злится на сотрудника.

Собственно не понятно, что именно Вам не нравится в этом коде (например, если весь проект усыпан кодом, который так и так не под MS работать не будет, то почему бы и нет).

Как Вам уже писали Выше, Вам следовало бы предложить свой вариант.

Здравствуйте, sch, Вы писали:

sch>Если человек вменяем, но не понимает что это будет работать только в debug, то объяснить ему.

Проверять срыв стека в release ? Наверное, такое может встретиться либо в чем-то очень экзотичном, либо если это защита от взлома (тогда учитывая примое использование таких констант — плохая защита от взлома). Да и не стал бы автор топика писать тут этот код в таком случае...

Здравствуйте, Corvin, Вы писали:

C>Здравствуйте, Corvin, Вы писали:

C>>Здравствуйте, slavo, Вы писали:

S>>>Что делать с сотрудником, который написал такой код?
S>>>

S>>>if((int)ici_os.a_top[-1] == 0xfdfdfdfd || *(int*)ici_os.a_top[-1] == 0xcccccccc))
S>>>{
S>>>  ici_error = "stack error";
S>>>  goto fail;
S>>>}
S>>>

C>>А он случайно не из Индии родом?

C>Помнится, у нас на проекте один из директоров конторы-заказчика как-то написал такую обертку:


C>

C>void* UMW_Alloc(int nSize)
C>{
C>   if (nSize > MAX_MEMORY)
C>      return malloc(MAX_MEMORY);
C>   else
C>      return malloc(nSize);
C>}
C>

C>Тот, кто первый на это наткнулся, долго искал причину.

ОООооо, зачёт! Задумку оценил Мужик был реально крут!!

LordMAD wrote:

> У меня создается впечатление, что Вы просто не знаете что это за
> константы и от этого начинаете злится на сотрудника.
>
> Собственно не понятно, что именно Вам не нравится в этом коде (например,
> если весь проект усыпан кодом, который так и так не под MS работать не
> будет, то почему бы и нет).

Присоединяюсь к недоумению.
без сомнения, нужен контекст.
Все т.н. "антипаттерны", приведённые в топике -- полная ерунда.
Всё зависит от обстоятельств.

Здравствуйте, LordMAD, Вы писали:

LMA>Здравствуйте, StevenIvanov, Вы писали:

SI>>Антипаттерны:
SI>>- магические числа

LMA>Эти значения программистами узнаются лучше, чем какие-то имена для них.

SI>>- платформозависимость
SI>>- использование goto
SI>>- (если код С++) — использование C-style cast

LMA>Сами такие антипаттерны придумали или подсказал кто?

А для вас антипаттенны существуют только в книжках?

По определению:

A design pattern is a "solution to a problem in context"

В случае антипаттернов имеем то же самое — "антирешение" проблемы в некотором контексте.

Если данный код не отладочный и внесен в релиз (в противном случае обсуждение кода лишено смысла) такое решение свидетельствует о крайне непрофессиональном подходе.

Здравствуйте, StevenIvanov, Вы писали:

SI>>>Антипаттерны:
SI>>>- магические числа

LMA>>Эти значения программистами узнаются лучше, чем какие-то имена для них.

SI>>>- платформозависимость
SI>>>- использование goto
SI>>>- (если код С++) — использование C-style cast

LMA>>Сами такие антипаттерны придумали или подсказал кто?

SI>А для вас антипаттенны существуют только в книжках?

SI>По определению:

SI>

SI>A design pattern is a "solution to a problem in context"

SI>В случае антипаттернов имеем то же самое — "антирешение" проблемы в некотором контексте.

SI>Если данный код не отладочный и внесен в релиз (в противном случае обсуждение кода лишено смысла) такое решение свидетельствует о крайне непрофессиональном подходе.

Что касается первого из приведенных Вами антипаттернов, то тут мы имеем дело с величинами, знакомыми каждому, поэтому нет смысла использовать именованные константы. Не надо доводить дело до абсурда: если переменная содержит оценку по математике, то не надо заводить именованные константы для 2, 3, 4 и 5 — и так всем понятно — что есть что. В данном конкретном случае использование именованных констант привело бы к запутыванию, а не улучшению читабельности (при работе с дампами памяти эти константы обычно не заменяются на имена, поэтому узнаются программистами сходу).

Что касается остальных "антипаттернов", то они вовсе не являются антипаттернами. Это нормально: писать платформозависимый код (не нормально как раз тратить ресурсы на написание платформонезависимого кода "про запас"; было бы очень интересно увидеть, как бы Вы написали платформонезависимую проверку на срыв стека — в данном случае имеет место нечто похожее именно на это, IMHO); использовать goto в низкоуровневом коде; использовать C-style cast в коде на C++, если повсеместно в проекте используется старый код на C.

Другими словами, перечисленные вами 3 "антипаттерна", мягко говоря, не являются общепризнанными антипаттернами. Тот же goto наличествует в современных языках не потому, что авторы этих языков — дураки, а потому, что есть случаи, когда goto — то, что надо, несмотря на то, что такие случае редки. И пока автор топика не приведет свой вариант кода, который на его взгляд будет более элегантен, нет причин считать, что данный код плох.

Здравствуйте, slavo, Вы писали:

S>увольняю

да, что-то вы, slavo, погорячились
Непонятно, зачем спрашивать на форуме, когда всё равно делаете по своему вразрез с мнением публики.

Здравствуйте, LordMAD, Вы писали:

LMA>...
SI>>В случае антипаттернов имеем то же самое — "антирешение" проблемы в некотором контексте.

SI>>Если данный код не отладочный и внесен в релиз (в противном случае обсуждение кода лишено смысла) такое решение свидетельствует о крайне непрофессиональном подходе.

LMA>Что касается первого из приведенных Вами антипаттернов, то тут мы имеем дело с величинами, знакомыми каждому, поэтому нет смысла использовать именованные константы. Не надо доводить дело до абсурда...

Вот мне эти величины ни о чем не говорят. Хоть и программирую на С/C++ вот почти уже 5 лет (в основном linux+gcc). И печальный опыт сопровеждения проектов в которых есть такие места тоже есть
Опять же — нет проблем если написано "для себя", на минутку, но если выкладывать такое в качестве финального варианта...


LMA>Что касается остальных "антипаттернов", то они вовсе не являются антипаттернами. Это нормально: писать платформозависимый код (не нормально как раз тратить ресурсы на написание платформонезависимого кода "про запас"; было бы очень интересно увидеть, как бы Вы написали платформонезависимую проверку на срыв стека — в данном случае имеет место нечто похожее именно на это, IMHO); использовать goto в низкоуровневом коде; использовать C-style cast в коде на C++, если повсеместно в проекте используется старый код на C.

Безусловно это нормально. Когда это имеет смысл. Однако, скажем так, не всегда удобно иметь дело с кодом, в котором россыпью "int8, uint32, int16" и на другой платформе компилятор плюется пачками ворнингов.

LMA>Другими словами, перечисленные вами 3 "антипаттерна", мягко говоря, не являются общепризнанными антипаттернами. Тот же goto наличествует в современных языках не потому, что авторы этих языков — дураки, а потому, что есть случаи, когда goto — то, что надо, несмотря на то, что такие случае редки. И пока автор топика не приведет свой вариант кода, который на его взгляд будет более элегантен, нет причин считать, что данный код плох.

По поводу goto — нет проблем, если код на С. Да еще если код-стайл требует оформления в стиле single entry-single exit. В случае с С++ такой код выглядит несколько неопрятно. Как правило в С++ных программах goto встречается в километровых функциях, что так же "антипаттерн". Хотя может быть и кому-то нравится сопровождать такой код

Хотя касаемо топика судить довольно сложно, т.к. не ясен контекст кода.

Здравствуйте, slavo, Вы писали:

S>Что делать с сотрудником, который написал такой код?
S>

S>if((int)ici_os.a_top[-1] == 0xfdfdfdfd || *(int*)ici_os.a_top[-1] == 0xcccccccc))
S>{
S>  ici_error = "stack error";
S>  goto fail;
S>}
S>

Нормальный код. Единственное, нужно магические числа задефайнить и скобки поставить
*((int*)ici_os.a_top[-1]).
Не было сказано на каком языке эта программа. Если на С, то проблем вообще нет. Это для С нормально.
Ну и добавить немного коментов.

p.s. люди, которые сразу кидаются увольнять таких сотрудников или сильно бить палками — реально неадекватны.

Здравствуйте, StevenIvanov, Вы писали:

SI>Вот мне эти величины ни о чем не говорят. Хоть и программирую на С/C++ вот почти уже 5 лет (в основном linux+gcc). И печальный опыт сопровеждения проектов в которых есть такие места тоже есть

Код явно специфичен для MS. Плохо это или хорошо — зависит от проекта. Никаких антипаттернов тут быть не может.

SI>Опять же — нет проблем если написано "для себя", на минутку, но если выкладывать такое в качестве финального варианта...

Не любой код может быть написан с поддержкой нескольких платформ. Может ли так быть написан этот код — вопрос остается открытым.

LMA>>Что касается остальных "антипаттернов", то они вовсе не являются антипаттернами. Это нормально: писать платформозависимый код (не нормально как раз тратить ресурсы на написание платформонезависимого кода "про запас"; было бы очень интересно увидеть, как бы Вы написали платформонезависимую проверку на срыв стека — в данном случае имеет место нечто похожее именно на это, IMHO); использовать goto в низкоуровневом коде; использовать C-style cast в коде на C++, если повсеместно в проекте используется старый код на C.

SI>Безусловно это нормально. Когда это имеет смысл. Однако, скажем так, не всегда удобно иметь дело с кодом, в котором россыпью "int8, uint32, int16" и на другой платформе компилятор плюется пачками ворнингов.

Всё на свете должно использоваться только когда оно имеет смысл. А еще — лучше быть здоровым и богатым, чем бедным и больным. Только какое это имеет отношение к приведенным Вами антипаттернам и данному топику?

SI>По поводу goto — нет проблем, если код на С. Да еще если код-стайл требует оформления в стиле single entry-single exit. В случае с С++ такой код выглядит несколько неопрятно.

По-вашему низкоуровневый код на C++ не пишут?

SI>Как правило в С++ных программах goto встречается в километровых функциях, что так же "антипаттерн". Хотя может быть и кому-то нравится сопровождать такой код

Если Вы считаете, что раз Вам встречается "как правило" код "с душком", то это не значит, что у всех так.

SI>Хотя касаемо топика судить довольно сложно, т.к. не ясен контекст кода.

Ну и смысл тогда разбрасываться модными словечками вроде "антипаттерн"? В надежде, что даже сломанные часы дважды в сутки показывают правильно время?

Здравствуйте, slavo, Вы писали:

S>увольняю

Не знаю откуда такая ассоциация, но почему-то напомнило ветку из соседнего форума http://rsdn.ru/Forum/Message.aspx?mid=3092000&only=1

Здравствуйте, LordMAD, Вы писали:

LMA>Что касается первого из приведенных Вами антипаттернов, то тут мы имеем дело с величинами, знакомыми каждому, поэтому нет смысла использовать именованные константы.

Уже через полгода даже сам автор обычно не может вспомнить, что же это за "знакомые каждому" константы. А если константа используется неоднократно по коду? А если ее потом поменять захочется? (К примеру, MS вдруг захочет в новой версии компилятора другим значением стек заполнять)

Кстати, ты число "пи" тоже в программах циферками пишешь?

LMA>Не надо доводить дело до абсурда: если переменная содержит оценку по математике, то не надо заводить именованные константы для 2, 3, 4 и 5 — и так всем понятно — что есть что.

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

LMA>В данном конкретном случае использование именованных констант привело бы к запутыванию, а не улучшению читабельности (при работе с дампами памяти эти константы обычно не заменяются на имена, поэтому узнаются программистами сходу).

В промышленном программировании нет "данных конкретных случаев". Есть общие подходы, которые просто тупо работают, независимо от случая. Например, правило, что _все_ (то есть абсолютно все) константы в программе надо именовать. Тот, кому придется за тобой поддерживать твой код, меньше всего захочет разбираться в твоих персональных "конкретных случаях".

Здравствуйте, slavo, Вы писали:

S>>>Что делать с сотрудником, который написал такой код?
S>>>

S>>>if((int)ici_os.a_top[-1] == 0xfdfdfdfd || *(int*)ici_os.a_top[-1] == 0xcccccccc))
S>>>{
S>>>  ici_error = "stack error";
S>>>  goto fail;
S>>>}
S>>>

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

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

Ну на счёт мэджиков понятно — "слышал звон, да не знает где он", "главное успеть к релизу", ... .
Интересно вот что: почему [-1] ??? Ведь там же что угодно может быть без всяких расстрелов. Или там ДОЛЖНЫ лежать нормальные данные?