1. C++ / Говнокод #4223

    +165

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    12. 12
    13. 13
    14. 14
    15. 15
    16. 16
    17. 17
    18. 18
    19. 19
    20. 20
    21. 21
    22. 22
    23. 23
    24. 24
    25. 25
    26. 26
    27. 27
    28. 28
    29. 29
    30. 30
    31. 31
    32. 32
    33. 33
    34. 34
    35. 35
    36. 36
    37. 37
    38. 38
    39. 39
    40. 40
    41. 41
    42. 42
    43. 43
    44. 44
    45. 45
    46. 46
    47. 47
    48. 48
    49. 49
    50. 50
    51. 51
    52. 52
    53. 53
    54. 54
    55. 55
    56. 56
    57. 57
    58. 58
    59. 59
    60. 60
    61. 61
    62. 62
    63. 63
    64. 64
    65. 65
    66. 66
    67. 67
    68. 68
    69. 69
    70. 70
    71. 71
    72. 72
    73. 73
    74. 74
    75. 75
    case  KEY_F9: {
    			if ( !strcmp( chlist->gettype(), "ethernet" ) ) {
    				/* Редактор канала Ethernet */
    				char buf[0x100];
    				chlist->gen_path_chan( CHANCONFDIR, "/m.cfg", buf );
    				//
    				tethcfgedit* edit = new tethcfgedit( getscr(), buf );
    				edit->layer = layer;
    				edit->Run();
    				delete edit;
    			} else if ( !strcmp( chlist->gettype(), "gprs" ) ) {
    				/* Редактор канала GPRS */
    				char buf[0x100];
    				chlist->gen_path_chan( CHANCONFDIR, "", buf );
    				//
    				tgprscfgedit* edit = new tgprscfgedit( getscr(), buf );
    				edit->layer = layer;
    				edit->Run();
    				delete edit;
    			} else if ( !strcmp( chlist->gettype(), "gsm" ) ) {
    				/* Редактор канала GSM */
    				char buf[0x100];
    				chlist->gen_path_chan( CHANCONFDIR, "", buf );
    				//
    				tgsmcfgedit* edit = new tgsmcfgedit( getscr(), buf );
    				edit->layer = layer;
    				edit->Run();
    				delete edit;
    			} else if ( !strcmp( chlist->gettype(), "gsppp" ) ) {
    				/* Редактор канала GS (пакетный) */
    				char buf[0x100];
    				chlist->gen_path_chan( CHANCONFDIR, "", buf );
    				//
    				tgspppcfgedit* edit = new tgspppcfgedit( getscr(), buf );
    				edit->layer = layer;
    				edit->Run();
    				delete edit;
    			} else if ( !strcmp( chlist->gettype(), "gs" ) ) {
    				/* Редактор канала GS (прямой) */
    				char buf[0x100];
    				chlist->gen_path_chan( CHANCONFDIR, "", buf );
    				//
    				tgscfgedit* edit = new tgscfgedit( getscr(), buf );
    				edit->layer = layer;
    				edit->Run();
    				delete edit;
    			} else if ( !strcmp( chlist->gettype(), "radio" ) ) {
    				/* Редактор канала радиомодема */
    				char buf[0x100];
    				chlist->gen_path_chan( CHANCONFDIR, "", buf );
    				//
    				tradiocfgedit* edit = new tradiocfgedit( getscr(), buf );
    				edit->layer = layer;
    				edit->Run();
    				delete edit;
    			} else if ( !strcmp( chlist->gettype(),"cbs" ) ) {
    				/* Редактор канала Ethernet */
    				char buf[0x100];
    				chlist->gen_path_chan( CHANCONFDIR,"",buf );
    				//
    				tcbscfgedit* edit=new tcbscfgedit( getscr(),buf );
    				edit->layer = layer;
    				edit->Run();
    				delete edit;
    			} else if ( !strcmp( chlist->gettype(), "ptsn" ) ) {
    				/* Редактор канала GSM */
    				char buf[0x100];
    				chlist->gen_path_chan( CHANCONFDIR, "", buf );
    				//
    				tptsncfgedit* edit = new tptsncfgedit( getscr(), buf );
    				edit->layer = layer;
    				edit->Run();
    				delete edit;
    			}
    ......

    Кусок case'a, где запускается редактор настроек соотвествующего канала связи. Код из одной встроенной железки.

    Запостил: vollmond, 11 Сентября 2010

    Комментарии (53) RSS

    • очередная копипаста
      Ответить
      • В 61 строке перед buf нет пробела!
        Подозрительно...
        Ответить
        • И вокруг присваивания тоже. Похоже "ручная" сборка :))
          Ответить
    • нисмишно

      вдобавок какой суровый кодер решил тип канала связи хранить как строку?
      Ответить
      • > какой суровый кодер
        Суровый сибирский
        Ответить
      • Чем плоха строка, кроме того, что не позволяет использовать switch-case ?
        Особенно, если учесть, что конфигурация хранится в текстовом файле.

        Вообще - решения, вырванные из контекста задачи - сложно оценить чем они и как обоснованы.
        Ответить
    • показать все, что скрытоОдно только `!strcmp(...` - это уже сразу если не говнокод, то как минимум индусокод. Дальше можно не смотреть.
      Ответить
      • А чем не угодил "!strcmp(..." ?
        Между прочим это сравнение на равенство строк.
        Ибо strcmp() возвращает 0, когда строки равны.
        Так что индусокод - это для незнающих как минимум.
        Ответить
        • Прекрасно. Никто не сомневается в том, что это сравнение работает правильно. Но это не означает, что так надо делать. Стилистически это - классический индусокод, никакой разумной читабельностью не обладающий. Хрестоматийный пример индусокода, кстати. Именно для "знающих" :)

          Функция `strcmp` - функция relational-сравнения, а не equality-сравнения. Не надо пытаться делать из нее функцию equality-сравнения такими говнотрюками. Приставке "не" нет места в стравнении на равенство и совать туда этот `!` не надо. Если вы не "кул хацкер", пишите как все нормальные люди `if (strcmp(..., ...) == 0)`. А за `if (!strcmp(..., ...))` - сразу в отпуск в Индию.
          Ответить
          • Не надо тут заниматься калиграфией.
            И искать калОграфию там, где ее нет.
            P.S. В Гоа я бы не отказался съездить :)
            Ответить
            • Надо, надо! К тому же я сразу сказал, что `!strcmp(...` - это не говнокод, а скорее индусокод. Т.е. все не так страшно... Т.е. страшно, но не так :)
              Ответить
              • Зачем бояться очевидных вещей?
                В любой отрасли есть вещи очевидные для людей с опытом,
                и совершенно не очевидные для новичков.
                При этом эквивалентность strcmp() == 0 и !strcmp() относится как раз к такому случаю.
                Не нравится такой подход, напиши strequal() - будет очевиднее.
                Вот пример: while(*dest++ = *src++). Покажи его новичку: скажет "что за бред?".
                А это всего лишь копирование строк.
                Ответить
                • Эм... Причем здесь очевидность? 95% процентов говнокода - совершенно очевидные вещи! В коде выше - тоже соврешенно ничего неочевидного нет. Тем не менее он является говнокодом. Говнокодовость к очевидности/неочевидности никакого отношения не имеет.

                  Речь идет но об очевидности, а об индусокодовсти/говнокодовости. '!strcmp` - конструкция индусокодовая. "Для людей с опытом" :)
                  Ответить
                  • прежде чем навязывать правила, перечитай определения. индусокод - неоправданно неочевидное решение (таковое, из-за неопытности автора), сбивающее с толку даже опытных.
                    другая функция, которая equal, наверняка вызывает strcmp
                    Ответить
                  • > ничего неочевидного нет
                    Хорошая иллюстрация :)) Пытаетесь заразить неприязнью к отрицаниям?
                    Ответить
                • Есть две условных "школы" построения условных выражений в С (ну и соответствнно в С++)

                  1. Строгая: без явного сравнения допускаются только значения с выраженно булевской семантикой, как например
                  char c;
                  ...
                  if (isalpha(c))
                  ...

                  но ни в коем случае не
                  int *ptr;
                  ...
                  if (ptr)
                  ...


                  2. Нестрогая: без явного сравнения допускаются значения с выраженно булевской семантикой, а также значения, неявно конверитируемые в булевскую семантику с сохранением натуральных соотвествий типа "есть - 1, нет - 0", "равны - 1, не равны - 0", "успешно - 1, не успешно - 0" и т.п. В рамках этого подхода допускается код типа
                  int *ptr;
                  ...
                  if (ptr)
                  ...

                  и т.п.

                  Сравнения же типа `strcmp`, где равенство выражается через 0, при использовании в булевском контексте (где 0 имеет отрицательный смысл - "ложь") - это либо индусокод, либо яркий признак "кул хацкеров" только что "прочитавших учебник". "Натренироваться" (мягко выражаясь) читать такое, разумеется, можно, как можно "натренироваться" читать любой говнокод. Но смысл?
                  Ответить
                  • показать все, что скрытопо секрету: это никто не дочитал
                    Ответить
                    • О, об этом не волнуйтесь! Дочитали, и очень многие. У меня в это опыта поболее вашего будет и, поверьте: дочитывают, заучивают наизусть и еще просят!

                      А то, что есть экземпляры, которые "не дочитали"... Конечно же есть! Откуда, вы думаете, говонокод берется? Вот именно от этих недочитывающих (извините за очевидность)!
                      Ответить
                      • показать все, что скрытоудивительный человек, на упрек в нудятине нудите еще больше.
                        у вас там два пункта и постскриптум из копипасты и пяти тыщ точек. от такого у нормальных людей только депрессия развивается
                        Ответить
                  • >Сравнения же типа `strcmp`, где равенство выражается через 0, при использовании в булевском контексте (где 0 имеет отрицательный смысл - "ложь") - это либо индусокод, либо яркий признак "кул хацкеров" только что "прочитавших учебник".
                    Функция возвращает разность двух строк. Может, всё дело в нелогичном названии? Думаю, strdiff (от difference) подошло бы больше.

                    С попыткой обосрать "Нестрогую" школу не согласен.
                    Ответить
                    • Никакой попытки "обосрать нестрогую школу" у меня нет. Вариант `!strcmp` не попадает под "нестрогую школу" именно потому, что возврат 0 из `strcmp` имеет "положительную" семантику - строки равны. Это притиворечит требованиям "нестрогой школы". Именно по этой причине `strcmp` не попадает ни под строгую, ни под нестрогую школу и всегда, с точки зрения грамотного кодирования, требует явного сравнения результата в коде.
                      Ответить
                      • А чем это будет лучше? Чем прямое сравнение лучше отрицания? Тот же 0, та же положительная семантика. С другой стороны, любое выражение вида if (!(...)) у меня, например, прочно ассоциируется с проверкой на 0.
                        Ответить
                        • Совершенно верно - с проверкой на ноль. А что нас на самом деле интересует? А интересует нас проверка на равенство строк. Цепочка "ноль обозачает равенство строк" - неестественна, потому что, как я сказал выше, "ноль" имеет ярко выраженную отрицательную коннотацию, а "равенство" - положительную . Именно поэтому для повышения читабельности кода рекомендуется записывать данную цепочку явно.
                          Ответить
                          • >Цепочка "ноль обозачает равенство строк" - неестественна
                            Языки программирования - вообще сугубо искусственный продукт.
                            Допустим, 0 - приводился бы к true, а не 0 - к false.
                            strcmp() стал бы естественнее, по-вашему мнению?
                            Ответить
                            • В данном контексте? `if (strcmp(..., ...))` для отлова равенства строк? Конечно естественнее! Тут и спору никакого быть не может.
                              Ответить
                              • Жертвовать общим случаем из-за частного не разумно.
                                Если хочется большей "естественности" - std::string и operator== в помощь.
                                Ответить
                                • >Жертвовать общим случаем из-за частного не разумно.

                                  а вот ваш с++ божок Степанов прямо противоположного мнения =)
                                  Ответить
                  • if (isalpha(c)) тут как-то выпадает.
                    Уж лучше было б про if (ptr != NULL ) упомянуть, если действительно хочется разные "школы" показать.
                    Я вот до сих пор не могу понять, зачем навязывать писать strcmp() == 0, вместо !strcmp() если это приводит к совершенно одинаковым результатам с точки зрения компилятора, но при этом второй вариант даже на пару символов короче записывается. Смиритесь с тем, что 0 неявно приводится к false, и многие этим пользуются. Особенно учитывая что в Си явного булевского типа не было. А городить вместо одной strcmp() три разных функции strless(), strgreater() и strequal(), возвращающие "истинные" логические результаты, глупо.
                    Ответить
                    • `isalpha` никуда не выпадает именно потому, что иллюстрирует пример явно продекларированной булевской семантики, при том, что возвращаемый ей результат имеет тип `int`.

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

                      Сссылка на "одинаковые результаты с точки зрения компилятора" мне не ясна тоже. Я вел речь о стилистических свойствах кода. Какая разница, что там имеет место с точки зрения компилятора? Говнокод - это более чем в половине случаев именно стилистически кривой код. Пример - перед вами. Код работает правильно? Да. Нарушает правила языка? Нет. Т.е. с точки зрения компилятора там все прекрасно. Будете по этой причине утверждать, что это не говнокод?
                      Ответить
                      • isalpha выпадает в силу отсутствия парного примера.
                        Стилистические свойства кода в любом случае будут опираться на семантические.
                        А они в данном случае четко прописаны в стандартной, между прочим, библиотеке.
                        Где четко сказано: strcmp() возвращает 0 для равных строк.
                        И хоть ты тресни, иначе уже не будет.
                        Ответить
                        • >isalpha выпадает в силу отсутствия парного примера.

                          а isbukva?
                          Ответить
              • Дык тут весь код в данном примере не говнокод, а быдлокод. Влобешник писал, а не индус.

                Что до if(!strcmp) — думаю, нормальная конструкция. При небольшом опыте читается на раз, хотя есть у меня знакомые — молодые — которые задефайнили Ok в 0 и пишут strcmp == Ok. Или, соответственно, наоборот. Но это все-таки не индусятина, а стилистика.
                Ответить
                • Можно и bool equal(..., ...) { return strcmp(..., ...) == 0; } заинлайнить при желании. Но не нужно.
                  Ответить
          • Ну, если это: if (!strcmp(..., ...)) считать говнотрюком, может, стоит перейти, скажем, на Delphi?
            Ответить
            • а вот в Паскале за такое бросят в сортир :)
              Ответить
            • Ну зачем на Delphi? Может стоит поучиться писать нормальный код на С?
              Ответить
              • Тогда зачем вам C? Паскаль гораздо более читабелен и не позволяет таких вольностей.
                Ответить
                • Любой язык соответствующего уровня позволяет массу разнообразных вольностей. Поэтому попытки сравнения языков с точки зрения "позволения вольностей" мне не понятны.
                  Ответить
                  • Споров было бы меньше :)
                    if StrCmp(..., ...) = 0 then ...
                    До явного преобразования к boolean не у каждого говнокодера руки дойдут.
                    Ответить
      • Даун, открой учебник и пойми, что это равнозначные вещи.. Не для студентов и прочих недолюдей код писался, все кто знает С - понимает.
        Ответить
    • Кстати, еще одна потенциальная кривость данного кода - работа с самими редакторами. Потенциальная - потому что из кода не ясно, были ли разнообразные редакторы потомками одного полиморфного класса-редактора. Если были, то тогда разуменее было бы в ветках лишь создавать редакоры, запоминая результат в указателе базового типа
      tcfgedit* edit = NULL; // `tcfgedit` - полиморфный базовый класс
      ...
      // Ветвление
      ...
        edit = new tgscfgedit( getscr(), buf ); // `tgscfgedit` - конкретный редактор
      ...
      // Ветвление закончилось 
      
      // Общий для всех код
      assert(edit != NULL);
      edit->layer = layer;
      edit->Run();
      delete edit;

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

      Я бы предположил, что существовал, т.к. "аффар" занимается динамическим созданием/удалением объектов-редакторов через `new/delete` (вместо использования локального объекта). По-видимому, автор просто скопировал способ создания/удаления объекта из чужого кода, не понимая, что динамическое создание/удаление было применено там именно по причине полиморфности класса.
      Ответить
      • А вариант, что так было или рассчитываетсялось, что так будет, но забыли, не приходит в голову?
        Ответить
      • нету тут никакого полиморфизма. данный код по смыслу напоминает вот что:
        {
        int *i = new int;
        *i = 666;
        delete i;
        }


        у меня к Гуру есть вопрос:
        if(0 == 0)
        имеет положительную или отрицательную семантику?
        Ответить
        • Код вырван из контекста, как Вы узнали, что нет полиморфизма? У всех объектов есть общий предок.
          Ответить
          • >У всех объектов есть общий предок.
            Тогда зачем используется chlist->gettype() ?
            Сделали бы виртуальные функции.
            UPD: немного не туда посмотрел :) Но всё же
            Ответить
            • У всех объектов edit'ов. Чанлист - отдельная фигня. http://govnokod.ru/4223#comment47278 вменяемое решение, имхо:)
              Ответить
              • http://govnokod.ru/4223#comment47278 лишь частично устраняет убогость говнокода

                более изящное решение - сделать Run() виртуальной функ. и заюзать std::map. и пусть общий предок будет <generic_edit> и set() тоже виртуальна.
                using namespace std;
                typedef map<string, generic_edit *> type_map_t;
                type_map_t type_map;
                
                type_map.insert(make_pair("ethernet", new tethcfgedit));
                type_map.insert(make_pair("gprs", new tgprscfgedit));
                [...]
                
                type_map_t::iterator it = type_map.find(chlist->gettype());
                generic_edit *edit = it->second;
                edit->set(getscr(), buf, layer);
                edit->Run();
                Ответить
          • допустим есть общий предок и что?

            в приведенном коде статистический и динамический типы у *edit всегда совпадают
            Ответить
            • Ну так на то он и говнокод. А надо было делать, чтобы не совпадали.
              Ответить
        • Так о том и речь. Полиморфизма нету. А надо было сделать с полиморфизмом. При условии, конечно, что иерархия классов разработана соответствующим образом (т.е. с полиморфизмом). Мы эту иерерхию не видим, поэтому судить трудно.

          Если в иерархии полиморфизма нет, то тогда делать тут `new/delete` вообще не надо было. Надо было объявлять обычные локальные объекты и не лезть в динамическую память вообще.
          Ответить

    Добавить комментарий