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

    +1002

    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
    void TExternalIOBuffer::swap(TExternalIOBuffer& Buffer)
{
	ASSERT(typeid(Buffer)==typeid(TExternalIOBuffer));
	const TExternalIOBuffer CurrentBuffer=*this;
	const TExternalIOBuffer OtherBuffer=Buffer;
	Buffer.~TExternalIOBuffer();
	::new((void*)&Buffer) TExternalIOBuffer(CurrentBuffer);
	this->~TExternalIOBuffer();
	::new((void*)this) TExternalIOBuffer(OtherBuffer);
};

const TExternalIOBuffer& TExternalIOBuffer::operator=(const TAbstractIOBuffer& Buffer)
{
	this->~TExternalIOBuffer();
	::new((void*)this)TExternalIOBuffer(Buffer);
	return *this;
};

    Большой проект, попало в релиз.

    Запостил: Говногость, 16 Апреля 2012

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

    • ava someone 16.04.2012 14:10 # 0

      Мда.

      Использовать std::swap религия не позволяет? Или определять копирующий конструктор через оператор =, а не наоборот?

      Зачем всё это шаманство с placement new? Тем более криво реализованное и неработающее. Особенно умиляет двойной вызов конструктора для временного объекта на стеке.
      Ответить

      • ava Говногость 16.04.2012 15:09 # −1

        > Зачем всё это шаманство с placement new?
        Понятно, что это нужно переписать, но сейчас нет времени этим заниматься.

        > Тем более криво реализованное и неработающее. Особенно умиляет двойной вызов конструктора для временного объекта на стеке.
        Можно поподробнее по обоим пунктам? У нас ошибка не проявляется, ибо заказчик уже использует прогу.

        Ну и может ещё какие проблемы углядели за этим кодом? Огласите весь список пожалуйста. Часть проблем я вижу, ну а может я что-то проглядел.
        Ответить

        • ava someone 16.04.2012 16:27 # +1

          Моя ошибка, оно работающее. Но это из серии "удаление гланд через анус". Два (!) временных объекта, для каждого из которых дважды вызывается конструктор...

          void TExternalIOBuffer::swap(TExternalIOBuffer& Buffer)
{
	ASSERT(typeid(Buffer)==typeid(TExternalIOBuffer));
	std::swap(this, Buffer);
};


          или, если религия не позволяет использовать стандартную библиотеку...

          void TExternalIOBuffer::swap(TExternalIOBuffer& Buffer)
{
	ASSERT(typeid(Buffer)==typeid(TExternalIOBuffer));
	TExternalIOBuffer temp = Buffer;
	Buffer = *this;
	*this = temp;
};


          Оба варианта проще, чем то, что в коде, и наверняка быстрее (один конструктор, один деструктор и два оператора присваивания вместо четырёх конструкторов и четырёх деструкторов).

          Мораль: не пишите чрезмерно заумный код. Пишите то, что имеете в виду.
          Ответить

          • ava Говногость 16.04.2012 17:25 # 0

            Самое плохое, что этот код не безопасен с точки зрения исключений. Также нет проверки в operator= assert(Buffer != this).
            Ответить

            • ava someone 16.04.2012 17:59 # 0

              Я считаю, что в operator = должен быть не assert, а проверка типа (if (this == &Buffer) { return *this; }.

              Присвоить переменную примитивного типа самой себе можно, и это ноп. Чем объекты хуже? :)
              Ответить

              • ava Говногость 16.04.2012 18:26 # 0

                > проверка типа (if (this == &Buffer) { return *this; }.
                Не, явно признак какой-то незапланированной ситуации. Так что лучше ассерт.
                Ответить

                • ava Steve_Brown 16.04.2012 19:02 # 0

                  Ну мало ли где возникнет... Скажем, сдвиг массива на N элементов в вырожденном случае N=0 вряд ли должен выдавать ассерт.
                  Ответить

                  • ava Говногость 16.04.2012 19:06 # 0

                    Лишние O(n) бесполезных операций? Не нужны. Лучше замечу и уберу.
                    А if (this == &Buffer) { return *this; } нужен на всякий случай для релиза.
                    Ответить

                    • ava Steve_Brown 17.04.2012 09:54 # +1

                      А, ну если и ассерт (для отладки), и проверка (для сохранения корректной работы в особых случаях), тогда, конечно, да.
                      Ответить

                      • ava Говногость 19.04.2012 23:07 # 0

                        На самом деле не хочу говно трогать, чтобы не завоняло. В багтрекере этого нет, а стало быть за него не заплатят.
                        Ответить

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