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

    +139

    1. 1
    2. 2
    3. 3
    4. 4
    5. 5
    6. 6
    7. 7
    8. 8
    #define MIN(i1, i2) (i1 < i2 ? i1 : i2)

int mr_word_compare(const char* r1, int s1, const char* r2, int s2)
{
    int l1 = strchr(r1, ' ') - r1;
    int l2 = strchr(r2, ' ') - r2;
    return strncmp(r1, r2, MIN(l1, l2));
}

    пердложенный вариант исправления #4093 (http://govnokod.ru/4093)

    Запостил: vayerx, 25 Августа 2010

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

    • ava degeron 25.08.2010 09:36 # +2

      Зачем в функцию s1 и s2 передаются, если они не используются?
      Ответить

    • ava Altravert 25.08.2010 10:13 # 0

      Стремный какой-то способ работы с указтелями.. Тем более, что
      strchr(r1, ' ') - r1

      выходит затратнее, чем
      int i = 0;
while (*p1 != ' ')
{
      ++i;
      ++ p1;
}

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

      • ava Setry 25.08.2010 11:07 # −2

        безусловно циклом лучше:
        1) здесь нет проверки, что если пробела вообще нет в строке
        2) используя цикл можно вести поиск не до пробела, а скажем примерно так:
        #define is_separator(x) x == ' ' || x == ','
        ...
        if (is_separator(*p))break;

        а функция целиком будет выглядеть примерно так:
        #define is_separator(x) x == ' ' || x == ','

        int foo (char *str1, char *str2)
        {
        char *s1 = str1, *s2 = str2;
        int len = 0;
        while (*s1 && *s2)
        {
        if (is_separator(*s1++))break;
        if (is_separotor(*s2++))break;
        ++len;
        }
        return strncmp (str1, str2, len);
        }


        ну и макрос MIN это просто жесть)
        нельзя такие использовать
        Ответить

        • ava Altravert 25.08.2010 11:14 # 0

          Юзай 
          тэги
          . А так молодец)
          Ответить

        • ava Kornew 25.08.2010 12:00 # 0

          если уж на то пошло:
          int foo (const char *str1, const char *str2) {
   while ( *str2 == *str2 &&
            *str1 != '\0' && *str1 != ' ' //все нужные разделители
          )
   {
      *str1++;
      *str2++;
   }
   retrun (*str1 > *str2)?1:(*str1 == *str2?0:-1); //аналогично strcmp()
}


          или так, в более универсальном виде (с набором разделителей в параметре spr):
          int foo (const char *str1, const char *str2, const char *spr) {
   while ( *str2 == *str2 && 
            *str1 != '\0' && !strchr(spr, *str1)
          )
   {
      *str1++;
      *str2++;
   }
   retrun (*str1 > *str2)?1:(*str1 == *str2?0:-1); //аналогично strcmp()
}



          накой хрен вам сдалась та строковая функция, если вы на прямую работаете со строкой?: вам лишние циклы по приколу?...
          Ответить

          • ava Kornew 25.08.2010 12:10 # 0

            правка: return (а не retrun)...

            хотя там по-ходу много фейлов: сейчас уточним =Ъ
            Ответить

          • ava Kornew 25.08.2010 12:24 # +1

            fixed:

            с фиксированными разделителями
            int foo(const char *str1, const char *str2)
{
   while( *str1++ == *str2++ &&
            *str1 != '\0' && *str1 != ' ' // другие разделители
           );
   return (*(--str1) > *(--str2))?1:(*str1 == *str2?0:-1);
}


            с набором разделителями в параметре:
            int foo(const char *str1, const char *str2, const char *spr)
{
   while( *str1++ == *str2++ &&
            *str1 != '\0' && !strchr(spr, *str1)
           );
   return (*(--str1) > *(--str2))?1:(*str1 == *str2?0:-1);
}
            Ответить

            • ava Govnoeb 25.08.2010 18:44 # +1

              твой код начинает вонять при
              сonst char s0[] = { 0 };
	const char s1[] = { 0 };
        const char delims[] = { ' ', 0 };

         printf("%d\n", foo(s0, s1, delims));


              тем более, return можно записать проще:
              return *--str1 - *--str2;


              мои соболезнования
              Ответить

              • ava Kornew 25.08.2010 19:26 # 0

                да, ты прав, в этом случае берётся str1[-1], в следствии чего результат будет не верный...
                необходима заплатка..

                кстати, на счёт ретурна тоже интересно, сразу не додумал =(
                Ответить

                • ava Govnoeb 25.08.2010 23:13 # +1

                  не так. у тебя ведь постфиксный инкремент: указатели просто проскочат '\0' и вылезут за границы массивов

                  лечится перестановкой условий:
                  assert(str1 && str2);
while(*str1 && *str1 != delim && *str1 == *str2) {
  ++str1; ++str2;
}

return *str1 - *str2;
                  Ответить

                  • ava Kornew 26.08.2010 01:16 # 0

                    даа, чот котелок сёдня ошибки выдаёт =(
                    Ответить

              • ava Kornew 25.08.2010 19:39 # 0

                кстати, гавном начинает вонять уже на параметрах:

                foo(NULL, NULL, NULL);

                xD
                Ответить

          • ava Altravert 25.08.2010 12:47 # 0

            Не аналогично strcmp. Она сравнивает все символы. strcmp("ab", "b") даст -1.
            UPD: Пизжу, вроде ты прав)) А в первом варианте это был пиздееец)
            Ответить

            • ava Kornew 25.08.2010 13:10 # 0

              та да, первый вариант не удачный, там ошибок довольно много было =)
              Ответить

        • ava Govnoeb 25.08.2010 14:19 # 0

          при подстановке макроса, s1 и s2 будут увеличены дважды

          неси зачетку
          Ответить

          • ava Setry 25.08.2010 19:34 # 0

            да, ты прав
            причем я макрос MIN покритиковал и сам наступил на те же грабли=)
            Ответить

    • ava Govnoeb 25.08.2010 14:49 # −1

      проблема с MIN() в том, что тогда функция возможно будет сравнивать подстроки. т.е. след код вернет 0:

      const char s0[] = { 'g', 'o', 'v', 'n', 'o', ' ', 0 };
const char s1[] = { 'g', 'o', 'v', 'n', 'o', 'e', 'b', ' ', 0 };

printf("%d\n", mr_word_compare(s0, 0, s1, 0));


      мой вариант:
      int mr_word_compare(const char *r1, const char *r2, const char delim)
{
  const char *p;

  const int l1 = (p = strchr(r1, delim))?p - r1:strlen(r1);
  const int l2 = (p = strchr(r2, delim))?p - r2:strlen(r2);
    
  const int res = strncmp(r1, r2, MIN(l1, l2));
    
  return (res)?res:l1 - l2;
}
      Ответить

      • ava Altravert 25.08.2010 15:24 # +1

        Ты возвращаешь разницу длины?
        То есть, нет, если строки равны то ты возвращаешь разницу длины строк?
        Ответить

        • ava Govnoeb 25.08.2010 18:57 # 0

          да
          Ответить

          • ava Altravert 25.08.2010 19:15 # 0 

            int q = strcmp("a", "ba");
int qq = strcmp("ab", "b");

            Проверь. Длина не при чем.
            Ответить

            • ava Govnoeb 25.08.2010 23:18 # 0

              причем. mr_word_compare() по сути есть strncmp().

              проверь: 
              assert(strncmp("abcd", 2) == strncmp("abc", 2));


              так из-за MIN()
              Ответить

      • ava Kornew 25.08.2010 16:46 # +1

        4 цикла с полным проходом по строкам...
        кстати, если строки не имеют символов: '\0' и delim, то будет красотища неописуемая
        и еще 1 цикл для сравнения участка строки...

        гламурно, чо =)
        ----------------------
        как выше было показано:
        это делается более оптимальным способом (1 проход по строкам) и в зависимости от функциональности:
        - 1 проверка на символ-разделитель
        - или 1 цикл проверок на набор разделителей
        Ответить

      • ava Govnoeb 25.08.2010 18:58 # 0

        вот блядство

        сразу после входа в функцию надо добавить 
        assert(r1 && r2);
        Ответить

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