current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 3 down vote favorite

I am familiar with the simple use of QSqlQuery::prepare() and QSqlQuery::bindValue() as in:

QSqlQuery query;
query.prepare("SELECT name, address FROM members WHERE id = :id");
query.bindValue(":id", member_id);
query.exec();

In this scenario I show a list of people (members) to the user, all with a check-box in front. The user can simply check which members to delete. So now, the number of :id are variable.

Currently, I solve this issue in quite a untidy fashion. All the id's checked people will be put in an array of integers called checked_items_ids and then I simply for-loop that array:

QString prepare_query = QString("DELETE FROM members WHERE id = ").append(checked_items_ids[0]);

for(int i = 1; i < checked_items_ids.count(); i++)
    prepare_query.append(QString(" OR id = %1").arg(checked_items_ids[i]));

QSqlQuery query(prepare_query);
query.exec();

Is there a 'tidier' way to do this? Using bindValue() or anything that I might be overlooking?

share|improve this question
up vote 2 down vote accepted

I'm not the expert for QSqlQuery, so my answer might be slightly off the mark. I see nothing wrong in not using prepare() and bindValue(). It also seems to me a bit "odd" to use these in a DELETE statement. As I said, I'm not the expert on this, there may be other guys that can shed some light on this aspect.

I see two cosmetic issues:

  1. the way you use checked_items_ids, first time with null argument, second time iterating. A QStringList can help in constructing the SQL statement.
  2. setting the SQL statement in the constructor. Setting it in the exec() function might be more "natural".

These are cosmetic issues only. They do, however, improve readability of your code.

QStringList querylist;
for(auto id : checked_items_ids)
    querylist << QString("id = %1").arg(id);
QString querystring = QString("DELETE FROM members WHERE ") +
                  querylist.join(" OR ");

QSqlQuery query;
query.exec(querystring);
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.