query is not mean to be a copied. Use move construction instead

129 Views Asked by At

I've been doing a code where a database is manipulated, elements are saved and edited by a Qsqlite database and query, so the way I've used is to pass query by parameter I don't know how bad it is but every time it goes By parameter I get this warning: " QSqlQuery is deprecated: is not mean to be copied: use move construction instead" and I wanted to know the correct way to do it, I show the MainWindow constructor, (the warning is indicated in the main).

MainWindow::MainWindow(QWidget *parent)
    : QMainWindow(parent)
    , ui(new Ui::MainWindow)
{   

    ui->setupUi(this);
    char dirNow[1024];

    db = QSqlDatabase::addDatabase("QSQLITE");
    QString dirfull = QString(_getcwd(dirNow, 1024)) + "\\inventario.db";
    db.setDatabaseName(dirfull);

    if(!db.open()){
        qDebug() << db.lastError().text();
    }

    model = new QSqlQueryModel();

    QSqlQuery query(db);



    if(!query.exec("CREATE TABLE IF NOT EXISTS articulo (codigo INTEGER NOT NULL, nombre VARCHAR(55) NOT NULL, unidades INTEGER NOT NULL, "
                   "categoria VARCHAR(55) NOT NULL, pais VARCHAR(55) NOT NULL, precio DOUBLE NOT NULL, foto VARCHAR(200) NOT NULL, id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT) ")){
            QMessageBox::information(this, "Error", query.lastError().text());
    }

    if(!query.exec("CREATE TABLE IF NOT EXISTS categorias(valor VARCHAR(55) NOT NULL) ")){
        QMessageBox::information(this, "Error", query.lastError().text());
    }

    //query.prepare("DELETE FROM articulo WHERE  = 1");
    //query.addBindValue("");

    "Warning is here: " id = imprimirArticulos(query);

    "Warning is here: " QObject::connect(ui->registrarBtn, &QPushButton::clicked, this, [=]()->void{registrarArticulo(query); });
    QObject::connect(ui->addImagenBtn, &QPushButton::clicked, this, [=]()->void{subirFoto();});
    "Warning is here: " QObject::connect(ui->buscarBtn, &QPushButton::clicked, this, [=]()->void{filtroArticulos(query);});

    "Warning is here: " imprimirCategorias(query);
    "Warning is here: " QObject::connect(ui->categoriasCBox, &QComboBox::currentIndexChanged, this, [=]()->void{agregarCategorias(query);});

    model->setQuery(std::move(query));
}
2

There are 2 best solutions below

6
user10 On

They want to make QSqlQuery object move only so only one query will exist.

id = imprimirArticulos(query) //creates copy of query

What you need to do is to move query:

id = imprimirArticulos(std::move(query))

If you try to use query again after moving it will be undefined behavior so you have to move it back from function to main to use it again in imprimirCategorias for example.

1
Diego Ferruchelli On

Expanding on fg's answer and edited: when you call a function passing by value your query object, the compiler makes a copy of it (as it does with any stack allocated object). The exact kind of copy depends, as usual, on the copy constructor of the class. For this class, it seems to be a shallow copy. Because of this, when the query object has associated resources (for instance, when you prepared the query and associated bind variables with it), all instances (the original one and the copy used by each function) will point to the same "internal" resources. And that may cause problems if (and only if) you change something in one instance of query, affecting the others.

It's not exactly the same case, but you can read it here: https://doc.qt.io/qt-6/qsqlquery-obsolete.html

QSqlQuery cannot be meaningfully copied. Prepared statements, bound values and so on will not work correctly, depending on your database driver (for instance, changing the copy will affect the original). Treat QSqlQuery as a move-only type instead.

When using std::move(query), you obtain a "xvalue expression" of query, which can be directly passed to some library overloads. These overloads are actually move constructors (and, because they "may assume the argument is the only reference to the object", they will very probably "steal" the content of the object, leaving query in a "undefined behavior" state, not anymore usable by the calling function). See: https://en.cppreference.com/w/cpp/utility/move

The key point here is: if the query object is NOT modified (in your main code or in the called functions), you may: a) just dismiss the warning and keep the code as it is; or b) use std::move() and create a new query object for each call. You can't (you shouldn't) reuse the original query after calling std::move() on it. On the contrary, if you DO modify the query object (maybe inside one of the functions), your only option is creating separate instances.