Loss of reference per parameter

Question

I have the following problem, when executing the method insereOP() the instances maq and cli lose their data.

So when I call the method atualizarOP, client and machine information do not exist, generating error in SQL command.

    ClienteDAO cliDAO(con);
    Cliente * cli = cliDAO.getCliente(3);
    MaquinaDAO maqDAO(con);
    Maquina * maq = maqDAO.getMaquina(1);
    OrdemDeProducao tmp("op001",cli,maq,1,10000,QDate(2014,9,1));
    OrdemDeProducaoDAO dialogDAO(con);
    if (dialogDAO.insereOP(tmp)){
        std::cout << "Inserido" << std::endl;
        tmp.setQuantidadeProgramada(50000);
        if(dialogDAO.atualizaOP(tmp)) {
            std::cout << "Atualizado" << std::endl;
        }
    }

Methods used:

Maquina *MaquinaDAO::getMaquina(int codigoMaquina) {
    Maquina * retorno = NULL;
    if(db.open()) {
        query = QSqlQuery(db);
        query.prepare("SELECT CodigoMaquina, NomeMaquina, StatusMaquina FROM Maquina WHERE CodigoMaquina = ?");
        query.addBindValue(codigoMaquina);
        if(!query.exec()){
            std::cout << query.lastError().text().toStdString() << std::endl;
            db.close();
            return retorno;
        } else {
            if (query.first())
                retorno = new Maquina(query.value(0).toInt(), query.value(1).toString(), query.value(2).toInt());
        }
        db.close();
    } else {
        std::cout << db.lastError().text().toStdString() << std::endl;
    }
    return retorno;
}

Cliente *ClienteDAO::getCliente(int codigoCliente) {
    Cliente * retorno = NULL;
    if(db.open()) {
        query = QSqlQuery(db);
        query.prepare("SELECT CodigoCliente, NomeCliente FROM Clientes WHERE CodigoCliente = ?");
        query.addBindValue(codigoCliente);
        if(!query.exec()){
            std::cout << query.lastError().text().toStdString() << std::endl;
            db.close();
            return retorno;
        } else {
            if (query.first())
                retorno = new Cliente(query.value(0).toInt(), query.value(1).toString());
        }
        db.close();
    } else {
        std::cout << db.lastError().text().toStdString() << std::endl;
    }
    return retorno;
}

bool OrdemDeProducaoDAO::insereOP(OrdemDeProducao op) {
    if(db.open()) {
        query = QSqlQuery(db);
        query.prepare("INSERT INTO OrdemdeProducao (op, CodigoCliente, CodigoMaquina, ordem, QuantidadeProgramada, datadeentrega) VALUES (?,?,?,?,?,?)");
        query.addBindValue(op.getOP());
        query.addBindValue(op.getCliente()->getCodigoCliente());
        query.addBindValue(op.getMaquina()->getCodigoMaquina());
        query.addBindValue(op.getOrdem());
        query.addBindValue(op.getQuantidadeProgramada());
        query.addBindValue(op.getDataDeEntrega());
        if(!query.exec()){
            std::cout << query.lastError().text().toStdString() << std::endl;
            db.close();
            return false;
        }
        db.close();
        return true;
    } else {
        std::cout << db.lastError().text().toStdString() << std::endl;
        return false;
    }
}

All project files can be found in the Github repository: https://github.com/LuanTavares/PointSW

What may be causing this loss of information ?

Answer 1

The guy OrdemDeProducao is created from pointers given to it in the constructor and desaloca these pointers in the destructor.

Your specific problem occurs when you copy an object of the type OrdemDeProducao because you didn’t implement a copy builder or an assignment operator. When this occurs the compiler generates a standard version of them for you that simply copy each field of the class.

In your case the effect of these generated functions will be that at the end of the copy two objects of type OrdemDeProducao point to the same objects of the type Cliente and Maquina. This is a problem since the two copies tried to delete the pointers they save by being destroyed causing the other copy to lose the objects it is pointing to.

This problem occurs precisely in the function call OrdemDeProducaoDAO::insereOP which receives as a parameter a copy of a OrdemDeProdução. At the end of the execution of this function the parameter op will be destroyed by calling your destructor which will delete the internal pointers of the object. By returning to the function main its object tmp will be with its internal pointers pointing to objects already deleted thus generating the problem when using them in the function OrdemDeProducaoDAO::atualizaOP.

To solve this problem you have to decide whether OrdemDeProducao takes control over the pointers passed to it in the constructor or just observes its state.

Should the objective of OrdemDeProducao Whether it’s just storing references to the customer and the machine that are objects under the control of another structure, you should not delete the pointers when destroying because they are not under your responsibility and will be deleted by the other structure. So just remove your constructor and the program will work.

I already case the objective of OrdemDeProducao is to take control under the objects of Cliente and Maquina passed as parameter then you should think what happens when you copy an object of this type. The two most common options are:

1. Objects of the type OrdemDeProducao when copied they must create new copies of the elements pointed by their pointers.

   OrdemDeProducao::OrdemDeProducao(const OrdemDeProducao& op)
   {
     this->cliente = new Cliente(*op.cliente);
     this->maquina = new Maquina(*op.maquina);
   }
   
   OrdemDeProducao& OrdemDeProducao::operator=(const OrdemDeProducao& op)
   {
     this->cliente = new Cliente(*op.cliente);
     this->maquina = new Maquina(*op.maquina);
   }
   

2. Objects of the type OrdemDeProducao shall not be copied.

   // Declare o construtor de cópia e o operador de atribuição
   // como privados para que o compilador gere uma
   // mensagem de erro quando forem utilizados. (pré C++11)
   class OrdemDeProducao
   {
     private:
     OrdemDeProducao::OrdemDeProducao(const OrdemDeProducao& op);
     OrdemDeProducao& OrdemDeProducao::operator=(const OrdemDeProducao& op);
   
     // Resto das definições das declarações da classe
     // ...
   };
   

If you choose the second option, you will have to modify all the functions you receive by value as objects of type parameter OrdemDeProducao so that they receive the same as references, constant or not depending on the case. Thus, their function OrdemDeProducaoDAO::insereOP would change to:

bool OrdemDeProducaoDAO::insereOP(const OrdemDeProducao& op);