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

    +172

    1. 1
    2. 2
    3. 3
    4. 4
    5. 5
    6. 6
    7. 7
    8. 8
    void Logger::LogString( std::string &s )
{
	if (s.size() > 4096) {
		// предотвращаем слишком длиные строки в логе.
		s.resize( 4096 );
	}
	m_LogStream << LogMessagePrefix() << s << std::endl;
}

    народ пару дней понадобилось найти почему XML сообщения, размером слегка больше обычного, Xerces не принимает. идеи иссякли - пока в лог не посмотрели и не нашли вот по такому (проиллюстрированому выше) чудо принципу работающий логгер.

    Запостил: Dummy00001, 10 Ноября 2010

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

    • ava TarasB 10.11.2010 15:52 # 0

      А почему магические числа?
      Ответить

      • ava Dummy00001 10.11.2010 16:29 # 0

        оригинальный код обвешан макросами до не могу. я бы мог оригинал запостить (без магических констант) но он просто нечитабелен. что к слову тоже съиграло свою роль: там из всего кода только строчки вокруг этого `s.resize()` и были читабельными.

        любимая фишка оригинального кода: к переменной iostream (которую я выше обозначил m_LogStream) обращатся через три-четыре вложеных глобальных макроса, которые уровень логгера проверяют и нужный стрим подсовывают. я это порывался раньше сюда запостить, но там такая туева хуча кода, что не даже смешно. а говно как раз что три уровня логгера через четыре уровня вложенности макросов (переливающие из пустого в порожнее) генерируется код который ожидает что в контексте есть переменная m_Log указывающая на объект логгера (у объекта логгера она тоже есть и поставлена в this) и у этой переменной есть несколько предопределенных полей, включая уровень лога и стримы. мне сложно представить что происходило в голове у человека/людей кто этот код писал (код есть часть продукта купленого у французской фирмы).
        Ответить

    • ava burdakovd 10.11.2010 21:11 # 0

      const std::string &s
      Ответить

      • ava absolut 10.11.2010 21:33 # 0

        тогда resize не получится
        Ответить

        • ava burdakovd 10.11.2010 21:43 # +3

          а нефиг ресайзить переданный аргумент. substr
          Ответить

          • ava burdakovd 10.11.2010 21:46 # +3

            Кстати, не очень шарю в фокусах C++, но кажется с сигнатурой "std::string&" LogString("Hello world") не будет работать, что должно было уже навести на размышления.
            Ответить

            • ava absolut 11.11.2010 00:21 # 0

              Ресайзить нефиг. согласен, и const бы это помог найти и устранить. А LogString("Hello world") видимо писали через 
              string str("Hello world");
LogString( str );
              или не логировали вовсе константных строк.
              Ответить

          • ava absolut 11.11.2010 21:36 # 0

            Можно и без substr(). Просто вывести нужное количество символов.
            Ответить

      • ava alexqc 11.11.2010 21:29 # 0

        А почему бы не std::string s (без ссылки)? Типа быстрее?
        Ответить

        • ava absolut 11.11.2010 21:42 # 0

          Потенциально да, так как не создается копии строки. Реально же - не факт, так как реализация std::string может и не копировать само строковое представление, пока не производится модификация строки.
          Ответить

          • ava alexqc 12.11.2010 10:11 # 0

            Ну так и я о том же. Во всех уважающих себя реализациях STL для строк используется подсчет ссылок и копирование при модификации. Т.е. вызов (std::string s) от (std::string &s) по скорости отличается лишь инкрементом счетчика (и декрементом при выходе). Однако часто встречается именно std::string &s. Но ИМХО, если человек ставит & - значит он о чем-то при этом думает (иначе б не заморачиваясь писал бы без &). Вот мне и интересны эти соображения. Неужели "экономия на спичках"?
            Ответить

            • ava absolut 12.11.2010 11:42 # +1

              Потому что далеко не все классы ведут себя подобно std::string. И как вы правильно заметили, даже не все реализации. Поэтому лучше не надеяться на реализацию, а использовать ссылку, вместо копирования потенциально тяжелого объекта.
              Ответить

            • ava Dummy00001 12.11.2010 13:21 # +1

              "Во всех уважающих себя реализациях STL для строк используется подсчет ссылок и копирование при модификации."

              Нет. Изначально это была популярная идея и были ее реализации в нескольких нишевых STLах.

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

              Как и та же оптимизация где `substr() const` возвращает только ссылку на подстроку не создавая новой строки (оригинал строки то не обязан быть const и другим потоком может уже начать меняться). Реализация оказалась сложнее чем предполагалось, и потеря производительности на всех остальных операциях была слишком большая. Проще и эффективней оказалось сделать более быстрый аллокатор памяти - без чего STL в целом тормозило бы до бесполезности.
              Ответить

    • ava Govnoeb 11.11.2010 16:07 # 0

      самый серьезный косяк, в том, что в логах должен отражатся тот факт, что строка "не влезла и была укорочена"
      Ответить

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