Смущает векторы классов: «Большая тройка», удаление класса после push_back и управление ресурсами - PullRequest
0 голосов
/ 30 ноября 2018

У меня есть такой класс:

class cSerialMessage
{
    public:
        cSerialMessage(const enumMessages type, std::string txmessage = "") {rxAnswer="";};

        // Get/Set Methods
        // ...
        bool AddIntParam();     // Stores an int as parameter for the message
        void BuildCompleteMessage() // Build the whole message following the protocol rules

    private:
        enumMessages m_type;
        std::string m_txmessage, m_rxanswer;
        // [...]
};

Затем у меня есть другой класс, который "управляет" очередью сообщений:

class cMsgManager
{
    public:
        cMsgManager();

        // [...]
        cSerialMessage* NewMessage(eMessages MSG);
        bool AddParameter(int Param);
        bool QueueMessage(bool FirstPosition);

    private:
        deque<cSerialMessage> cMS;
        cSerialMessage* tempMsg;
}

Соответствующая часть, где cSerialMessageсоздан и добавлен в очередь.

cSerialMessage* cMsgManager::NewMessage (eMessages MSG)
{
    // Begin the creation of a new message


    if (tempMsg!=nullptr) {
        // there was already a temporary message
        OutputDebugString ("NOPE!");
        return nullptr;
    }
    tempMsg = new cSerialMessage(MSG);
    return tempMsg;
}
//------------------------------------------------------------------------------
bool cMsgManager::AddParameter (int Param)
{
    if (tempMsg==nullptr) {
        // this means that NewMessage() was'nt called before.
        OutputDebugString ("NOPE!);
        return false;
    }
    return tempMsg->AddIntParam(Param);
}
//------------------------------------------------------------------------------
bool cMsgManager::QueueMessage(bool FirstPosition)
{
    if (tempMsg==nullptr) {
        // this means that NewMessage() was'nt called before.
        OutputDebugString ("NOPE!);
        return false;
    }

    // Build the final message
    tempMsg->BuildCompleteMessage();

    if (FirstPosition) {
        cMS.push_front(*tempMsg);
    }else{
        cMS.push_back(*tempMsg);
    }

    delete tempMsg;

    tempMsg=nullptr;
    return true;
}

Несмотря на все вопросы по этой теме ( это очень подробно ), я все еще в замешательстве.

Должен ли яудалить мою tempMsg?Копируется ли он в deque или, в конце концов, данные, указанные tempMsg, будут теми, к которым впоследствии будет получен доступ из deque?Или я должен создать конструктор копирования и оператор копирования-присваивания?

Ответы [ 2 ]

0 голосов
/ 30 ноября 2018
  1. Должен ли я удалить свою tempMsg?

Да, как написано, вы должны либо delete ваш tempMsg, либо повторно использовать один экземпляр дляжизнь программы (без внесения new единиц после первой).Повторное использование может потребовать очистки перед каждым повторным использованием, что вряд ли стоит.

Копируется ли он в deque или, в конце концов, данные, указанные tempMsg, будут теми, к которым позднее будет получен доступ из deque?

Скопировано;push_back получает ссылки, но документируется только для копирования или перемещения (и так как вы передали l-значение, оно будет копироваться).Вы можете сэкономить небольшую работу, выполнив cMS.push_front(std::move(*tempMsg));, чтобы очистить tempMsg (поскольку вы просто собираетесь удалить ее после, так что вы также можете сохранить копии).

Или я должен создать конструктор копирования и оператор присвоения копии?

Предполагая, что все члены вашего cSerialMessage правильно копируются (нет необработанных указателей или тому подобного)), и вы не определили никаких пользовательских операций копирования / перемещения или деструкторов, все должно быть в порядке;сгенерированный компилятором конструктор копирования будет работать нормально.cMsgManager с другой стороны, потребуется полное правило 3/5 конструкторов и деструкторов, потому что вы не использовали умный указатель или семантику значения для tempMsg.

Обратите внимание, что все это динамическое распределениебессмысленно / расточительно.Вы можете просто сделать tempMsg a cSerialMessage (не cSerialMessage*) и работать с ним по значению.Вы можете оставить его атрибутом экземпляра или просто NewMessage вернуть фактическое новое сообщение, а не хранить копию локально.Локальная копия превратит многопоточность или повторный ввод кода в кошмар, поэтому возвращение нового сообщения по значению и управление вызывающей стороной , вероятно, является лучшей идеей.

0 голосов
/ 30 ноября 2018

Должен ли я удалить tempMsg?Да, вы создали это, кто-то должен удалить это, и это должен быть вы.

Это скопировано в deque?Нет, то, на что указывает tempMsg, копируется в deque, но tempMsg все еще остается там, а объект, на который он указывает, все еще там, и поэтому его нужно удалить.

Нужно ли создаватьконструктор копирования и оператор копирования-назначения?Да, или отметьте их как удаленные, если вы довольны тем, что cMsgManager не может быть скопировано.

...