-1

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));
}
  • Do you think you will get a helpful answer faster if you also specify the exact line in the shown code that results in this compiler diagnostic, or if you leave it up to everyone else to guess which one it is? – Sam Varshavchik Feb 16 '23 at 15:06
  • 1
    This is my first question, sorry if I don't know how to do it very well. – Juan Diego Monroy Feb 16 '23 at 15:10
  • 1
    Before posting any questions on stackoverflow.com, everyone should take the [tour], read the [help], understand all the requirements for a [mre] and [ask] questions here. Not doing any of this results in a poor quality question almost every time. It then gets downvoted, closed, and then deleted. Repeated low-quality questions may result in a temporary ban from asking new questions. – Sam Varshavchik Feb 16 '23 at 15:31

2 Answers2

1

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.

user10
  • 266
  • 5
  • thank you so much, Your advice helped me, I will keep it in mind – Juan Diego Monroy Feb 16 '23 at 16:00
  • Sorry for the question but do you know how I return it to the main? – Juan Diego Monroy Feb 16 '23 at 16:43
  • @JuanDiegoMonroy When calling `std::move()`, you put the `query` object under "undefined behavior" state. You shouldn't call move() again (or directly use the `query` object again), except after recreating the `query` object. But you don't need to do it: you can still use (and pass and return, etc) the "xvalue expression" returned by the original call to `std::move()`. – Diego Ferruchelli Feb 16 '23 at 17:08
  • @JuanDiegoMonroy I've just understood **where** in your code is the actual copy of the `query` object (the copy that's triggering the warning): in each and every call that uses it (as a stack allocated object, it's passed by value). If you don't really change the "original" object (for instance, inside a loop), you have two options: ignore the warning (it doesn't really matter) or use `std::move()`. But if you do change the "original" object between calls, then you must create a new one each time (as calling `std::move()` will invalidate the "original"). – Diego Ferruchelli Feb 16 '23 at 18:09
  • Yes, I think that the best way to deal with the problem would be to ignore the warning, well, having multiple queries doesn't seem like the best thing to me, thanks for taking the time – Juan Diego Monroy Feb 16 '23 at 19:06
  • You welcome! Please check my heavily edited answer. – Diego Ferruchelli Feb 16 '23 at 19:44
0

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.

  • The interesting thing about `std::move()`, the _xvalue expressions_ and the _move constructors_ is that a called function can change a variable defined in the caller (that is, the content of the variable itself). This is different than passing a reference or pointer to an object (by any means) and changing its content. – Diego Ferruchelli Feb 16 '23 at 19:53