From f44360923e37f967fa01163b5ddbb6f5c0de8ddb Mon Sep 17 00:00:00 2001 From: Mark Brand Date: Mon, 19 Mar 2012 09:21:55 +0100 Subject: QSqlTM/QSqlRTM: improve style and readability General changes: const, scope, braces, hash[] for clarity, comment wording and spelling. QSqlRelationalTableModel::selectStatement() readability: Renamed private method. QVector.value() already defaults to null object value, so there is no point in handling this case explicitly. Alias rec for d->rec added more noise than clarity. Using "tables" list only adds an extra step. Simple concatenation does the trick. Deduplicate code for building table expression and JOIN condition. Change-Id: Ia52afaf3c3937a26595d5ae867982664002562d8 Reviewed-by: Honglei Zhang --- src/sql/models/qsqlrelationaltablemodel.cpp | 111 ++++++++++++---------------- src/sql/models/qsqltablemodel.cpp | 38 ++++------ 2 files changed, 63 insertions(+), 86 deletions(-) (limited to 'src') diff --git a/src/sql/models/qsqlrelationaltablemodel.cpp b/src/sql/models/qsqlrelationaltablemodel.cpp index 6744e9d55b..b5d98d56b7 100644 --- a/src/sql/models/qsqlrelationaltablemodel.cpp +++ b/src/sql/models/qsqlrelationaltablemodel.cpp @@ -259,7 +259,7 @@ public: : QSqlTableModelPrivate(), joinMode( QSqlRelationalTableModel::InnerJoin ) {} - QString relationField(const QString &tableName, const QString &fieldName) const; + QString fullyQualifiedFieldName(const QString &tableName, const QString &fieldName) const; int nameToIndex(const QString &name) const; mutable QVector relations; @@ -299,7 +299,7 @@ void QSqlRelationalTableModelPrivate::revertCachedRow(int row) int QSqlRelationalTableModelPrivate::nameToIndex(const QString &name) const { - QString fieldname = strippedFieldName(name); + const QString fieldname = strippedFieldName(name); int idx = baseRec.indexOf(fieldname); if (idx == -1) { // If the name is an alias we can find it here. @@ -520,8 +520,8 @@ QSqlRelation QSqlRelationalTableModel::relation(int column) const return d->relations.value(column).rel; } -QString QSqlRelationalTableModelPrivate::relationField(const QString &tableName, - const QString &fieldName) const +QString QSqlRelationalTableModelPrivate::fullyQualifiedFieldName(const QString &tableName, + const QString &fieldName) const { QString ret; ret.reserve(tableName.size() + fieldName.size() + 1); @@ -536,35 +536,25 @@ QString QSqlRelationalTableModelPrivate::relationField(const QString &tableName, QString QSqlRelationalTableModel::selectStatement() const { Q_D(const QSqlRelationalTableModel); - QString query; if (tableName().isEmpty()) - return query; + return QString(); if (d->relations.isEmpty()) return QSqlTableModel::selectStatement(); - QString tList; - QString fList; - QString where; - - QSqlRecord rec = d->baseRec; - QStringList tables; - const QRelation nullRelation; - // Count how many times each field name occurs in the record QHash fieldNames; QStringList fieldList; - for (int i = 0; i < rec.count(); ++i) { - QSqlRelation relation = d->relations.value(i, nullRelation).rel; + for (int i = 0; i < d->baseRec.count(); ++i) { + QSqlRelation relation = d->relations.value(i).rel; QString name; - if (relation.isValid()) - { + if (relation.isValid()) { // Count the display column name, not the original foreign key name = relation.displayColumn(); if (d->db.driver()->isIdentifierEscaped(name, QSqlDriver::FieldName)) name = d->db.driver()->stripDelimiters(name, QSqlDriver::FieldName); - QSqlRecord rec = database().record(relation.tableName()); + const QSqlRecord rec = database().record(relation.tableName()); for (int i = 0; i < rec.count(); ++i) { if (name.compare(rec.fieldName(i), Qt::CaseInsensitive) == 0) { name = rec.fieldName(i); @@ -572,21 +562,24 @@ QString QSqlRelationalTableModel::selectStatement() const } } } - else - name = rec.fieldName(i); - fieldNames.insert(name, fieldNames.value(name, 0) + 1); + else { + name = d->baseRec.fieldName(i); + } + fieldNames[name] = fieldNames.value(name, 0) + 1; fieldList.append(name); } - for (int i = 0; i < rec.count(); ++i) { - QSqlRelation relation = d->relations.value(i, nullRelation).rel; + QString fList; + QString conditions; + QString from = tableName(); + for (int i = 0; i < d->baseRec.count(); ++i) { + QSqlRelation relation = d->relations.value(i).rel; + const QString tableField = d->fullyQualifiedFieldName(tableName(), d->db.driver()->escapeIdentifier(d->baseRec.fieldName(i), QSqlDriver::FieldName)); if (relation.isValid()) { - QString relTableAlias = QString::fromLatin1("relTblAl_%1").arg(i); - if (!fList.isEmpty()) - fList.append(QLatin1String(", ")); - fList.append(d->relationField(relTableAlias, relation.displayColumn())); + const QString relTableAlias = QString::fromLatin1("relTblAl_%1").arg(i); + QString displayTableField = d->fullyQualifiedFieldName(relTableAlias, relation.displayColumn()); - // If there are duplicate field names they must be aliased + // Duplicate field names must be aliased if (fieldNames.value(fieldList[i]) > 1) { QString relTableName = relation.tableName().section(QChar::fromLatin1('.'), -1, -1); if (d->db.driver()->isIdentifierEscaped(relTableName, QSqlDriver::TableName)) @@ -594,54 +587,44 @@ QString QSqlRelationalTableModel::selectStatement() const QString displayColumn = relation.displayColumn(); if (d->db.driver()->isIdentifierEscaped(displayColumn, QSqlDriver::FieldName)) displayColumn = d->db.driver()->stripDelimiters(displayColumn, QSqlDriver::FieldName); - fList.append(QString::fromLatin1(" AS %1_%2_%3").arg(relTableName).arg(displayColumn).arg(fieldNames.value(fieldList[i]))); - fieldNames.insert(fieldList[i], fieldNames.value(fieldList[i])-1); + displayTableField.append(QString::fromLatin1(" AS %1_%2_%3").arg(relTableName).arg(displayColumn).arg(fieldNames.value(fieldList[i]))); + --fieldNames[fieldList[i]]; } + if (!fList.isEmpty()) + fList.append(QLatin1String(", ")); + fList.append(displayTableField); + + // Join related table + const QString tblexpr = relation.tableName().append(QLatin1Char(' ')).append(relTableAlias); + const QString relTableField = d->fullyQualifiedFieldName(relTableAlias, relation.indexColumn()); + const QString cond = tableField + QLatin1String(" = ") + relTableField; if (d->joinMode == QSqlRelationalTableModel::InnerJoin) { - // this needs fixing!! the below if is borken. - // Use LeftJoin mode if you want correct behavior - tables.append(relation.tableName().append(QLatin1Char(' ')).append(relTableAlias)); - if (!where.isEmpty()) - where.append(QLatin1String(" AND ")); - where.append(d->relationField(tableName(), d->db.driver()->escapeIdentifier(rec.fieldName(i), QSqlDriver::FieldName))); - where.append(QLatin1String(" = ")); - where.append(d->relationField(relTableAlias, relation.indexColumn())); + // FIXME: InnerJoin code is known to be broken. + // Use LeftJoin mode if you want correct behavior. + from.append(QLatin1String(", ")).append(tblexpr); + if (!conditions.isEmpty()) + conditions.append(QLatin1String(" AND ")); + conditions.append(cond); } else { - tables.append(QLatin1String(" LEFT JOIN")); - tables.append(relation.tableName().append(QLatin1Char(' ')).append(relTableAlias)); - tables.append(QLatin1String("ON")); - - QString clause; - clause.append(d->relationField(tableName(), d->db.driver()->escapeIdentifier(rec.fieldName(i), QSqlDriver::FieldName))); - clause.append(QLatin1String(" = ")); - clause.append(d->relationField(relTableAlias, relation.indexColumn())); - - tables.append(clause); + from.append(QLatin1String(" LEFT JOIN ")).append(tblexpr); + from.append(QLatin1String(" ON ")).append(cond); } } else { if (!fList.isEmpty()) fList.append(QLatin1String(", ")); - fList.append(d->relationField(tableName(), d->db.driver()->escapeIdentifier(rec.fieldName(i), QSqlDriver::FieldName))); + fList.append(tableField); } } - if (d->joinMode == QSqlRelationalTableModel::InnerJoin && !tables.isEmpty()) { - tList.append(tables.join(QLatin1String(", "))); - if(!tList.isEmpty()) - tList.prepend(QLatin1String(", ")); - } else - tList.append(tables.join(QLatin1String(" "))); - if (fList.isEmpty()) - return query; + return QString(); - tList.prepend(tableName()); - query.append(QLatin1String("SELECT ")); - query.append(fList).append(QLatin1String(" FROM ")).append(tList); + QString query = query.append(QLatin1String("SELECT ")); + query.append(fList).append(QLatin1String(" FROM ")).append(from); if (d->joinMode == QSqlRelationalTableModel::InnerJoin) { - qAppendWhereClause(query, where, filter()); + qAppendWhereClause(query, conditions, filter()); } else if (!filter().isEmpty()) { query.append(QLatin1String(" WHERE (")); query.append(filter()); @@ -795,8 +778,8 @@ QString QSqlRelationalTableModel::orderByClause() const return QSqlTableModel::orderByClause(); QString s = QLatin1String("ORDER BY "); - s.append(d->relationField(QLatin1String("relTblAl_") + QString::number(d->sortColumn), - rel.displayColumn())); + s.append(d->fullyQualifiedFieldName(QLatin1String("relTblAl_") + QString::number(d->sortColumn), + rel.displayColumn())); s += d->sortOrder == Qt::AscendingOrder ? QLatin1String(" ASC") : QLatin1String(" DESC"); return s; } diff --git a/src/sql/models/qsqltablemodel.cpp b/src/sql/models/qsqltablemodel.cpp index d8d691fa0c..0e5eba1a73 100644 --- a/src/sql/models/qsqltablemodel.cpp +++ b/src/sql/models/qsqltablemodel.cpp @@ -84,12 +84,9 @@ int QSqlTableModelPrivate::insertCount(int maxRow) const int cnt = 0; CacheMap::ConstIterator i = cache.constBegin(); const CacheMap::ConstIterator e = cache.constEnd(); - for (; - i != e && (maxRow < 0 || i.key() <= maxRow); - ++i) { + for ( ; i != e && (maxRow < 0 || i.key() <= maxRow); ++i) if (i.value().insert()) ++cnt; - } return cnt; } @@ -175,14 +172,12 @@ bool QSqlTableModelPrivate::exec(const QString &stmt, bool prepStatement, } } int i; - for (i = 0; i < rec.count(); ++i) { + for (i = 0; i < rec.count(); ++i) if (rec.isGenerated(i)) editQuery.addBindValue(rec.value(i)); - } - for (i = 0; i < whereValues.count(); ++i) { + for (i = 0; i < whereValues.count(); ++i) if (whereValues.isGenerated(i) && !whereValues.isNull(i)) editQuery.addBindValue(whereValues.value(i)); - } if (!editQuery.exec()) { error = editQuery.lastError(); @@ -363,7 +358,7 @@ QString QSqlTableModel::tableName() const bool QSqlTableModel::select() { Q_D(QSqlTableModel); - QString query = selectStatement(); + const QString query = selectStatement(); if (query.isEmpty()) return false; @@ -593,11 +588,11 @@ bool QSqlTableModel::updateRowInTable(int row, const QSqlRecord &values) emit beforeUpdate(row, rec); const QSqlRecord whereValues = d->primaryValues(row); - bool prepStatement = d->db.driver()->hasFeature(QSqlDriver::PreparedQueries); + const bool prepStatement = d->db.driver()->hasFeature(QSqlDriver::PreparedQueries); QString stmt = d->db.driver()->sqlStatement(QSqlDriver::UpdateStatement, d->tableName, rec, prepStatement); - QString where = d->db.driver()->sqlStatement(QSqlDriver::WhereStatement, d->tableName, - whereValues, prepStatement); + const QString where = d->db.driver()->sqlStatement(QSqlDriver::WhereStatement, d->tableName, + whereValues, prepStatement); if (stmt.isEmpty() || where.isEmpty() || row < 0 || row >= rowCount()) { d->error = QSqlError(QLatin1String("No Fields to update"), QString(), @@ -629,9 +624,9 @@ bool QSqlTableModel::insertRowIntoTable(const QSqlRecord &values) QSqlRecord rec = values; emit beforeInsert(rec); - bool prepStatement = d->db.driver()->hasFeature(QSqlDriver::PreparedQueries); - QString stmt = d->db.driver()->sqlStatement(QSqlDriver::InsertStatement, d->tableName, - rec, prepStatement); + const bool prepStatement = d->db.driver()->hasFeature(QSqlDriver::PreparedQueries); + const QString stmt = d->db.driver()->sqlStatement(QSqlDriver::InsertStatement, d->tableName, + rec, prepStatement); if (stmt.isEmpty()) { d->error = QSqlError(QLatin1String("No Fields to update"), QString(), @@ -660,15 +655,15 @@ bool QSqlTableModel::deleteRowFromTable(int row) emit beforeDelete(row); const QSqlRecord whereValues = d->primaryValues(row); - bool prepStatement = d->db.driver()->hasFeature(QSqlDriver::PreparedQueries); + const bool prepStatement = d->db.driver()->hasFeature(QSqlDriver::PreparedQueries); QString stmt = d->db.driver()->sqlStatement(QSqlDriver::DeleteStatement, d->tableName, QSqlRecord(), prepStatement); - QString where = d->db.driver()->sqlStatement(QSqlDriver::WhereStatement, - d->tableName, - whereValues, - prepStatement); + const QString where = d->db.driver()->sqlStatement(QSqlDriver::WhereStatement, + d->tableName, + whereValues, + prepStatement); if (stmt.isEmpty() || where.isEmpty()) { d->error = QSqlError(QLatin1String("Unable to delete row"), QString(), @@ -838,9 +833,8 @@ void QSqlTableModel::revertAll() Q_D(QSqlTableModel); const QList rows(d->cache.keys()); - for (int i = rows.size() - 1; i >= 0; --i) { + for (int i = rows.size() - 1; i >= 0; --i) revertRow(rows.value(i)); - } } /*! -- cgit v1.2.3