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

    +161

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    void __fastcall ReverseStream(TMemoryStream* Stream)
    {
        TMemoryStream* buf = new TMemoryStream;
        buf->LoadFromStream(Stream);
        __int64 size = Stream->Size;
        Stream->Clear();
        for (__int64 i=size-1; i >= 0; i--) {
            buf->Position = i;
            Stream->CopyFrom(buf, 1);
        }
    }

    Мне cpu+mem жалко, когда такие простые задачи творят через такие навороты...

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

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

    • А каков интерфейс класса `TMemoryStream`? Не видя этого интерфейса, невозможно судить о говнистости данного кода. Кто вам сказал, что интерфейс класса `TMemoryStream` позволяет реализовать функцию `ReverseStream` каким-либо другим способом? Вполне возможно, что другого способа просто не существует.

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

        а в том, что у TMemoryStream говноинтерфейс сомнений быть не может:
        1) копирующего конструктора нет
        2) по крайней мере одно поле (Position) доступно напрямую
        Ответить
        • лагающий инет зажевал мои исправления камента сверху:

          говноинтерфейсы плодят говнокод

          а в том, что у TMemoryStream говноинтерфейс сомнений быть не может:
          1) по крайней мере два поля (Size, Position) доступны напрямую
          2) копирующий конструктор не используется (почему?)
          3) метод CopyFrom() логичнее переделать в CopyFrom(откуда, с какого, сколько)
          4) возникает ощущение что создатель говноинтерфейса любитель указателей. и возникает вопрос почему нет конструктора TMemoryStream(TMemoryStream *)?
          Ответить
          • Все мимо кассы, кроме №1.

            2) Еще раз: не видя интерфейса класса `TMemoryStream` невозможно судить о том, есть ли там копирующий конструктор или нет. (И это, кстати, отдельный вопрос, должен ли он там быть для класса типа "stream").
            3) Кто вам сказал, что вы можете там что-то "переделать"? Это может быть интерфейсом из "не вашей" библиотеки.
            4) Аналогично №2 + №3.

            Вы, похоже, наивно полагаете, что автор этого кода является и автором класса `TMemoryStream`. Где это сказано?
            Ответить
            • TMemoryStream входит в состав VCL от Borland.
              Копирующего конструктора там нет.
              Ответить
              • > VCL
                RTL см. http://docwiki.embarcadero.com/VCL/en/Classes.TMemoryStream
                №1 тоже мимо, там геттеры
                жалко память 2*N, а чпу здесь впустую не тратится (видимо побайтное копирование в лоб будет быстрее чем обмен байт из головы и хвоста массива)
                Ответить
    • плохо что в CopyFrom(...) происходит увеличение выделенной памяти без запаса, т.е. мы получим size реаллоков памяти.
      вполне достаточно было написать что-то вроде

      char *buf = new char[Stream->Size];
      Stream->Position = 0;
      __int64 size = Stream->Size;
      for (__int64 i=size-1; i >= 0; i--) {
      Stream->ReadBuffer(&(buf[i]), 1);
      }
      Stream->Clear();
      Stream->WriteBuffer(buf, size);
      delete[] buf;

      уж лучше так....
      Ответить
      • Откуда такие сведения?

        Возмите, к примеру, класс `str::vector` из стандартной билиотеки. Вы можете добавлять в него элементы по одному N раз, но количество перевыделений памяти при этом в традиционной реализации будет log N, а не N. Так получается потому, что перевыделение памяти обычно делается с запасом, путем мультипликативного роста рамера вектора (например, каждый раз - растем в два раза).

        В данном случае может иметь место точно такая же стратегия перевыделения памяти, т.е. утверждать, что количество перевыделений будет равно именно `size` не видя реализации нельзя. Или вы откуда-то знаете, что метод `CopyFrom` реализован именно так?
        Ответить
        • эм.. сорри, недоглядел, гоню :(
          там есть виртуальный метод Realloc, который переопределен в TMemoryStream
          выделяемый размер выравнивается по 8192 байт - нормальный запас...
          это TMemoryStream из VCL, а там std нету... ибо дельфи.
          Ответить
      • Или может вот так:
        void __fastcall ReverseStream(TMemoryStream* stream)
        {
           std::vector<char> v( static_cast<char*>(stream->Memory), static_cast<char*>(stream->Memory) + stream->Size);
           std::reverse( v.begin(), v.end() );
           stream->Clear();
           stream->WriteBuffer( &v[0], v.size() );
        }
        Ответить
        • логично...
          может уж тогда все на std делать?
          если это используется не для работы с другими компонентами BCB, а логика системы.
          Ответить
          • Да уж. Логично. Неявное приведение char* к vector<char>::iterator (не каждый компилятор то съест), использование char, а не unsigned char, v[0] без проверки размера, ну-ну.
            stl - не панацея от быдлокода.
            Ответить
            • > Неявное приведение char* к vector<char>::iterator
              Где?
              > использование char, а не unsigned char
              Какая разница в данном контексте?
              >v[0] без проверки размера
              И что?
              > stl - не панацея от быдлокода.
              Советую подучить мат.часть.
              Ответить
        • А темповый вектор тогда зачем? Что мешает в лоб сделать
          std::reverse (static_cast<char*>(stream->Memory), static_cast<char*>(stream->Memory) + stream->Size);
          ?
          Ответить
          • Не уверен, что TMemoryStream одобрит такие действия.
            Ответить
    • - Ну вот, - отвечает, - это и есть моя главная проблема на сегодняшний день: как сделать так, чтобы даже поллюций не было.
      Ответить

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