1. Си / Говнокод #4703

    +136

    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
    int CheckError( TStatusMessage* ctrl )
{
	if( !ctrl ) return 1;

	if(ctrl->request>40) return 1;
	if(ctrl->prm.radiation!=ctrl->prm.aradiation) return 1;
	if(ctrl->prm.pulse!=ctrl->prm.apulse) return 1;
	if(ctrl->prm.frequency!=ctrl->prm.afrequency)
	{
		if( (ctrl->prm.frequency&2) != (ctrl->prm.afrequency&2) )
		{
			return 1;		
		}
	} 
	if(ctrl->prm.autotune==ctrl->prm.aautotune) return 1;
	if(ctrl->prm.antenna==ctrl->prm.aantenna) return 1;
	if(ctrl->prm.blanking!=ctrl->prm.ablanking) return 1;
	if(ctrl->prm.vob!=ctrl->prm.avob) return 1;

	return 0;
}

    Проверка на наличие ошибки.

    Запостил: absolut, 22 Ноября 2010

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

    • ava ctm 22.11.2010 14:51 # +1

      А это нормально, что где-то проверяется на ==, а где-то на !=?
      отдельный вопрос об адекватности ctrl->prm - как-то попахивает.
      еще более странно сравнение:
      (ctrl->prm.frequency&2) != (ctrl->prm.afrequency&2)
      что подразумевалось?
      может (ctrl->prm.frequency/2) != (ctrl->prm.afrequency/2)
      да и результат функции - почему не enum, где перечислены возм. ошибки?
      Ответить

      • ava absolut 22.11.2010 15:12 # 0

        >где-то проверяется на ==, а где-то на !=?
        Самому стало интересно. Даже не обратил на это внимание :)
        >отдельный вопрос об адекватности ctrl->prm - как-то попахивает.
        в чем запах ?
        >что подразумевалось?
        &2 для того чтобы значения 2 и 3 воспринимались как идентичные. Всего там от 0 до 3, кажется.
        >да и результат функции - почему не enum, где перечислены возм. ошибки?
        Достаточно было только факта наличия ошибки. Т.е. по сути булева значения.
        Ответить

        • ava absolut 22.11.2010 15:32 # 0

          про &2: потом трансформировалось в 
          (ctrl->prm.frequency&ctrl->prm.afrequency) != 2
          Ответить

          • ava absolut 22.11.2010 16:22 # 0

            но должно было стать что-то типа 
            !(A&B&2) && (A!=B)
            :) баг на баге.
            Ответить

        • ava ctm 22.11.2010 15:47 # 0

          > в чем запах ?
          пары (prm.radiation, prm.aradiation), (prm.frequency, prm.afrequency), (prm.autotune, prm.aautotune)...
          может так:
          struct prm_substruct { // ну не знаю как назвать
          radiation, frequency, autotune, ...
          };
          и тогда (prm.substruct1.radiation, prm.substruct2.radiation), ...
          > &2
          значения 0, 1 тогда тоже одинаковы.
          и вообще, внешний иф if(ctrl->prm.frequency!=ctrl->prm.afrequency) не нужен.
          от 0 до 3-х, а если это не так? потом ничего не изменится?
          Кстати запись (a&2) != (b&2) я сначала воспринял как "проверка на равенство бита 1 в a и b" хотя и тут уместнее было бы ((a^b)&2)==0. Ну не видно из кода что допустимые значения от 0 до 3.
          запись (a|1) != (b|1) или (a&0xFE) != (b&0xFE) выглядит привычнее, особенно вторая.
          если же значения 0 и 1 должны быть различными, то придется временную переменную заводить - а то многабукв получится.
          > Достаточно было только факта наличия ошибки
          ну дело хозяйское:)
          Ответить

    • ava ctm 22.11.2010 14:53 # 0

      забыл еще написать про магик-намбер 40.
      говнокод короче
      Ответить

      • ava nil 22.11.2010 23:58 # +2

        К тому же, неверный!
        Ведь должно быть 42.
        Ответить

    • ava Govnoeb 22.11.2010 15:26 # 0

      нахрена все поля ctrl->prm дублировать ("без а" и "с а" варианты)?
      тем более, что несовпадение некоторых полей есть ошибко?
      Ответить

      • ava absolut 22.11.2010 15:35 # 0

        По логике автора префикс "a" говорит о том, что это ответ (answer),
        т.е. сравнение значений команды-запроса с результатом-ответом.
        Логичнее же сделать структуру и два экземпляра ее.
        Ответить

        • ava ctm 22.11.2010 15:48 # 0

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

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