Skip to content

Commit d925bf0

Browse files
svettersvetter
committed
Make error messages for violated assertions of ItemIDs more useful
Closes #169
1 parent b78a6e2 commit d925bf0

20 files changed

+159
-95
lines changed

‎src/comp_tables/composite_column.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ QVariant ReferenceCompositeColumn::computeValueAt(BufferRowIndex rowIndex) const
473473
currentTable = (NormalTable*) referencedColumn->table;
474474

475475
// Find row index that contains the current primary key
476-
currentRowIndex = currentTable->getBufferIndexForPrimaryKey(key.forceValid());
476+
currentRowIndex = currentTable->getBufferIndexForPrimaryKey(FORCE_VALID(key));
477477
assert(currentRowIndex.isValid());
478478
}
479479

‎src/comp_tables/fold_composite_column.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ QSet<BufferRowIndex> FoldCompositeColumn::evaluateBreadcrumbTrail(BufferRowIndex
9292
QSet<ValidItemID> currentKeySet = QSet<ValidItemID>();
9393
for (const BufferRowIndex& bufferRowIndex : currentRowIndexSet) {
9494
ItemID key = firstColumn->getValueAt(bufferRowIndex);
95-
if (key.isValid()) currentKeySet.insert(key.forceValid());
95+
if (key.isValid()) currentKeySet.insert(FORCE_VALID(key));
9696
}
9797

9898
currentRowIndexSet.clear();
@@ -344,7 +344,7 @@ QStringList HikerListCompositeColumn::formatAndSortIntoStringList(QSet<BufferRow
344344

345345
// Check whether default hiker is set and get name if so
346346
if (defaultHiker->isNotNull()) {
347-
ValidItemID defaultHikerID = defaultHiker->get();
347+
ValidItemID defaultHikerID = VALID_ITEM_ID(defaultHiker->get());
348348
BufferRowIndex defaultHikerRowIndex = hikersTable->getBufferIndexForPrimaryKey(defaultHikerID);
349349
if (rowIndexSet.contains(defaultHikerRowIndex)) {
350350
QVariant content = contentColumn->getValueAt(defaultHikerRowIndex);

‎src/data/item_id.h

+65-1
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,20 @@ class ItemID {
5757
bool isValid() const;
5858
bool isInvalid() const;
5959

60+
private:
6061
int get() const;
62+
public:
6163
QVariant asQVariant() const;
6264

65+
private:
6366
ValidItemID forceValid() const;
6467

68+
public:
6569
void operator=(const ItemID& other);
70+
71+
friend class ItemIDPrivilegedFunctionAccessor;
72+
friend bool operator==(const ItemID& id1, const ItemID& id2);
73+
friend size_t qHash(const ItemID& key, size_t seed);
6674
};
6775

6876

@@ -74,13 +82,16 @@ class ItemID {
7482
* ID is valid.
7583
*/
7684
class ValidItemID : public ItemID {
77-
public:
85+
private:
7886
ValidItemID(int id);
7987
ValidItemID(QVariant id);
88+
public:
8089
ValidItemID(const ValidItemID& other);
8190

8291
void operator=(const ItemID& other) = delete;
8392
void operator=(const ValidItemID& other);
93+
94+
friend class ItemID;
8495
};
8596

8697

@@ -92,4 +103,57 @@ size_t qHash(const ItemID& key, size_t seed);
92103

93104

94105

106+
/**
107+
* A macro to get the integer index of an ItemID.
108+
*
109+
* Performs an assertion at caller level that the ItemID must be valid.
110+
* Always use this macro to make sure that assertions are performed at caller level first, to make
111+
* sure that violated assertion give a useful error message.
112+
*
113+
* @param item_id The ItemID to get the integer index of.
114+
* @return The integer index of the ItemID.
115+
*/
116+
#define ID_GET(item_id) (assert(item_id.isValid()), ItemIDPrivilegedFunctionAccessor::getValueForItemID(item_id))
117+
118+
/**
119+
* A macro to force an ItemID to be valid, turning it into a ValidItemID.
120+
*
121+
* Performs an assertion at caller level that the ItemID must be valid.
122+
* Always use this macro to make sure that assertions are performed at caller level first, to make
123+
* sure that violated assertion give a useful error message.
124+
*
125+
* @param item_id The ItemID to force to be valid.
126+
* @return The ValidItemID created from the ItemID.
127+
*/
128+
#define FORCE_VALID(item_id) (assert(item_id.isValid()), ItemIDPrivilegedFunctionAccessor::forceItemIDValid(item_id))
129+
130+
/**
131+
* A macro to create a ValidItemID from an int.
132+
*
133+
* Performs an assertion at caller level that the integer must produce a valid ItemID.
134+
* Always use this macro to make sure that assertions are performed at caller level first, to make
135+
* sure that violated assertion give a useful error message.
136+
*
137+
* @param integer The integer to create a ValidItemID from.
138+
* @return The ValidItemID created from the integer.
139+
*/
140+
#define VALID_ITEM_ID(integer) (assert(ItemID(integer).isValid()), ItemIDPrivilegedFunctionAccessor::forceItemIDValid(ItemID(integer)))
141+
142+
143+
/**
144+
* A class which serves as an indirection tool to make functions of ItemID which require assertions
145+
* publicly accessible, but in a way that makes it clear that they should not be used directly.
146+
*
147+
* Instead, the macros ID_GET, FORCE_VALID and VALID_ITEM_ID, which this is designed to work
148+
* with, should be used.
149+
*/
150+
class ItemIDPrivilegedFunctionAccessor
151+
{
152+
public:
153+
inline static int getValueForItemID (ItemID itemID) { return itemID.get(); }
154+
inline static ValidItemID forceItemIDValid (ItemID itemID) { return itemID.forceValid(); }
155+
};
156+
157+
158+
95159
#endif // ITEM_ID_H

‎src/db/associative_table.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ int AssociativeTable::getNumberOfMatchingRows(const Column* column, ValidItemID
142142
assert(column == column1 || column == column2);
143143
int numberOfMatches = 0;
144144
for (const QList<QVariant>* const bufferRow : buffer) {
145-
if (bufferRow->at(column->getIndex()) == primaryKey.get()) {
145+
if (bufferRow->at(column->getIndex()) == ID_GET(primaryKey)) {
146146
numberOfMatches++;
147147
}
148148
}
@@ -166,8 +166,8 @@ QSet<ValidItemID> AssociativeTable::getMatchingEntries(const Column* column, Val
166166
const Column* otherColumn = getOtherColumn(column);
167167
QSet<ValidItemID> filtered = QSet<ValidItemID>();
168168
for (const QList<QVariant>* const bufferRow : buffer) {
169-
if (bufferRow->at(column->getIndex()) == primaryKey.get()) {
170-
filtered.insert(bufferRow->at(otherColumn->getIndex()));
169+
if (bufferRow->at(column->getIndex()) == ID_GET(primaryKey)) {
170+
filtered.insert(VALID_ITEM_ID(bufferRow->at(otherColumn->getIndex()).toInt()));
171171
}
172172
}
173173
return filtered;

‎src/db/database.cpp

+39-39
Original file line numberDiff line numberDiff line change
@@ -363,19 +363,19 @@ Ascent* Database::getAscentAt(BufferRowIndex rowIndex) const
363363
const QList<QVariant>* row = ascentsTable->getBufferRow(rowIndex);
364364
assert(row->size() == ascentsTable->getNumberOfColumns());
365365

366-
ValidItemID ascentID = row->at(ascentsTable->primaryKeyColumn->getIndex());
367-
QString title = row->at(ascentsTable->titleColumn->getIndex()).toString();
368-
ItemID peakID = row->at(ascentsTable->peakIDColumn->getIndex());
369-
QDate date = row->at(ascentsTable->dateColumn->getIndex()).toDate();
370-
int perDayIndex = row->at(ascentsTable->peakOnDayColumn->getIndex()).toInt();
371-
QTime time = row->at(ascentsTable->timeColumn->getIndex()).toTime();
372-
int elevationGain = row->at(ascentsTable->elevationGainColumn->getIndex()).toInt();
373-
int hikeKind = row->at(ascentsTable->hikeKindColumn->getIndex()).toInt();
374-
bool traverse = row->at(ascentsTable->traverseColumn->getIndex()).toBool();
375-
int difficultySystem = row->at(ascentsTable->difficultySystemColumn->getIndex()).toInt();
376-
int difficultyGrade = row->at(ascentsTable->difficultyGradeColumn->getIndex()).toInt();
377-
ItemID tripID = row->at(ascentsTable->tripIDColumn->getIndex());
378-
QString description = row->at(ascentsTable->descriptionColumn->getIndex()).toString();
366+
ValidItemID ascentID = VALID_ITEM_ID(row->at(ascentsTable->primaryKeyColumn->getIndex()));
367+
QString title = row->at(ascentsTable->titleColumn->getIndex()).toString();
368+
ItemID peakID = row->at(ascentsTable->peakIDColumn->getIndex());
369+
QDate date = row->at(ascentsTable->dateColumn->getIndex()).toDate();
370+
int perDayIndex = row->at(ascentsTable->peakOnDayColumn->getIndex()).toInt();
371+
QTime time = row->at(ascentsTable->timeColumn->getIndex()).toTime();
372+
int elevationGain = row->at(ascentsTable->elevationGainColumn->getIndex()).toInt();
373+
int hikeKind = row->at(ascentsTable->hikeKindColumn->getIndex()).toInt();
374+
bool traverse = row->at(ascentsTable->traverseColumn->getIndex()).toBool();
375+
int difficultySystem = row->at(ascentsTable->difficultySystemColumn->getIndex()).toInt();
376+
int difficultyGrade = row->at(ascentsTable->difficultyGradeColumn->getIndex()).toInt();
377+
ItemID tripID = row->at(ascentsTable->tripIDColumn->getIndex());
378+
QString description = row->at(ascentsTable->descriptionColumn->getIndex()).toString();
379379

380380
QSet<ValidItemID> hikerIDs = participatedTable->getMatchingEntries(participatedTable->ascentIDColumn, ascentID);
381381
QList<Photo> photos = photosTable->getPhotosForAscent(ascentID);
@@ -399,14 +399,14 @@ Peak* Database::getPeakAt(BufferRowIndex rowIndex) const
399399
const QList<QVariant>* row = peaksTable->getBufferRow(rowIndex);
400400
assert(row->size() == peaksTable->getNumberOfColumns());
401401

402-
ValidItemID peakID = row->at(peaksTable->primaryKeyColumn->getIndex());
403-
QString name = row->at(peaksTable->nameColumn->getIndex()).toString();
404-
int height = row->at(peaksTable->heightColumn->getIndex()).toInt();
405-
bool volcano = row->at(peaksTable->volcanoColumn->getIndex()).toBool();
406-
int regionID = row->at(peaksTable->regionIDColumn->getIndex()).toInt();
407-
QString mapsLink = row->at(peaksTable->mapsLinkColumn->getIndex()).toString();
408-
QString earthLink = row->at(peaksTable->earthLinkColumn->getIndex()).toString();
409-
QString wikiLink = row->at(peaksTable->wikiLinkColumn->getIndex()).toString();
402+
ValidItemID peakID = VALID_ITEM_ID(row->at(peaksTable->primaryKeyColumn->getIndex()));
403+
QString name = row->at(peaksTable->nameColumn->getIndex()).toString();
404+
int height = row->at(peaksTable->heightColumn->getIndex()).toInt();
405+
bool volcano = row->at(peaksTable->volcanoColumn->getIndex()).toBool();
406+
int regionID = row->at(peaksTable->regionIDColumn->getIndex()).toInt();
407+
QString mapsLink = row->at(peaksTable->mapsLinkColumn->getIndex()).toString();
408+
QString earthLink = row->at(peaksTable->earthLinkColumn->getIndex()).toString();
409+
QString wikiLink = row->at(peaksTable->wikiLinkColumn->getIndex()).toString();
410410

411411
return new Peak(peakID, name, height, volcano, regionID, mapsLink, earthLink, wikiLink);
412412
}
@@ -427,11 +427,11 @@ Trip* Database::getTripAt(BufferRowIndex rowIndex) const
427427
const QList<QVariant>* row = tripsTable->getBufferRow(rowIndex);
428428
assert(row->size() == tripsTable->getNumberOfColumns());
429429

430-
ValidItemID tripID = row->at(tripsTable->primaryKeyColumn->getIndex());
431-
QString name = row->at(tripsTable->nameColumn->getIndex()).toString();
432-
QDate startDate = row->at(tripsTable->startDateColumn->getIndex()).toDate();
433-
QDate endDate = row->at(tripsTable->endDateColumn->getIndex()).toDate();
434-
QString description = row->at(tripsTable->descriptionColumn->getIndex()).toString();
430+
ValidItemID tripID = VALID_ITEM_ID(row->at(tripsTable->primaryKeyColumn->getIndex()));
431+
QString name = row->at(tripsTable->nameColumn->getIndex()).toString();
432+
QDate startDate = row->at(tripsTable->startDateColumn->getIndex()).toDate();
433+
QDate endDate = row->at(tripsTable->endDateColumn->getIndex()).toDate();
434+
QString description = row->at(tripsTable->descriptionColumn->getIndex()).toString();
435435

436436
return new Trip(tripID, name, startDate, endDate, description);
437437
}
@@ -452,8 +452,8 @@ Hiker* Database::getHikerAt(BufferRowIndex rowIndex) const
452452
const QList<QVariant>* row = hikersTable->getBufferRow(rowIndex);
453453
assert(row->size() == hikersTable->getNumberOfColumns());
454454

455-
ValidItemID hikerID = row->at(hikersTable->primaryKeyColumn->getIndex());
456-
QString name = row->at(hikersTable->nameColumn->getIndex()).toString();
455+
ValidItemID hikerID = VALID_ITEM_ID(row->at(hikersTable->primaryKeyColumn->getIndex()));
456+
QString name = row->at(hikersTable->nameColumn->getIndex()).toString();
457457

458458
return new Hiker(hikerID, name);
459459
}
@@ -474,10 +474,10 @@ Region* Database::getRegionAt(BufferRowIndex rowIndex) const
474474
const QList<QVariant>* row = regionsTable->getBufferRow(rowIndex);
475475
assert(row->size() == regionsTable->getNumberOfColumns());
476476

477-
ValidItemID regionID = row->at(regionsTable->primaryKeyColumn->getIndex());
478-
QString name = row->at(regionsTable->nameColumn->getIndex()).toString();
479-
int rangeID = row->at(regionsTable->rangeIDColumn->getIndex()).toInt();
480-
int countryID = row->at(regionsTable->countryIDColumn->getIndex()).toInt();
477+
ValidItemID regionID = VALID_ITEM_ID(row->at(regionsTable->primaryKeyColumn->getIndex()));
478+
QString name = row->at(regionsTable->nameColumn->getIndex()).toString();
479+
int rangeID = row->at(regionsTable->rangeIDColumn->getIndex()).toInt();
480+
int countryID = row->at(regionsTable->countryIDColumn->getIndex()).toInt();
481481

482482
return new Region(regionID, name, rangeID, countryID);
483483
}
@@ -498,9 +498,9 @@ Range* Database::getRangeAt(BufferRowIndex rowIndex) const
498498
const QList<QVariant>* row = rangesTable->getBufferRow(rowIndex);
499499
assert(row->size() == rangesTable->getNumberOfColumns());
500500

501-
ValidItemID rangeID = row->at(rangesTable->primaryKeyColumn->getIndex());
502-
QString name = row->at(rangesTable->nameColumn->getIndex()).toString();
503-
int continent = row->at(rangesTable->continentColumn->getIndex()).toInt();
501+
ValidItemID rangeID = VALID_ITEM_ID(row->at(rangesTable->primaryKeyColumn->getIndex()));
502+
QString name = row->at(rangesTable->nameColumn->getIndex()).toString();
503+
int continent = row->at(rangesTable->continentColumn->getIndex()).toInt();
504504

505505
return new Range(rangeID, name, continent);
506506
}
@@ -521,8 +521,8 @@ Country* Database::getCountryAt(BufferRowIndex rowIndex) const
521521
const QList<QVariant>* row = countriesTable->getBufferRow(rowIndex);
522522
assert(row->size() == countriesTable->getNumberOfColumns());
523523

524-
ValidItemID countryID = row->at(countriesTable->primaryKeyColumn->getIndex());
525-
QString name = row->at(countriesTable->nameColumn->getIndex()).toString();
524+
ValidItemID countryID = VALID_ITEM_ID(row->at(countriesTable->primaryKeyColumn->getIndex()));
525+
QString name = row->at(countriesTable->nameColumn->getIndex()).toString();
526526

527527
return new Country(countryID, name);
528528
}
@@ -632,7 +632,7 @@ QList<WhatIfDeleteResult> Database::removeRow_referenceSearch(QWidget* parent, b
632632
for (const Column* otherTableColumn : candidateNormalTable->getColumnList()) {
633633
if (otherTableColumn->getReferencedForeignColumn() != primaryKeyColumn) continue;
634634

635-
QList<BufferRowIndex> rowIndexList = candidateNormalTable->getMatchingBufferRowIndices(otherTableColumn, primaryKey.get());
635+
QList<BufferRowIndex> rowIndexList = candidateNormalTable->getMatchingBufferRowIndices(otherTableColumn, ID_GET(primaryKey));
636636
QSet<BufferRowIndex> rowIndexSet = QSet<BufferRowIndex>(rowIndexList.constBegin(), rowIndexList.constEnd());
637637

638638
affectedRowIndices.unite(rowIndexSet);
@@ -654,7 +654,7 @@ QList<WhatIfDeleteResult> Database::removeRow_referenceSearch(QWidget* parent, b
654654
const Column* affectedPrimaryKeyColumn = affectedTable->primaryKeyColumn;
655655

656656
for (const BufferRowIndex& rowIndex : rowIndices) {
657-
ValidItemID primaryKey = affectedPrimaryKeyColumn->getValueAt(rowIndex);
657+
ValidItemID primaryKey = VALID_ITEM_ID(affectedPrimaryKeyColumn->getValueAt(rowIndex));
658658
// Remove single instance of reference to the key about to be removed
659659
candidateNormalTable->updateCell(parent, primaryKey, affectedColumn, ItemID().asQVariant());
660660
}

‎src/db/normal_table.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ BufferRowIndex NormalTable::getBufferIndexForPrimaryKey(ValidItemID primaryKey)
6565
{
6666
BufferRowIndex index = BufferRowIndex(0);
6767
for (const QList<QVariant>* const bufferRow : buffer) {
68-
if (bufferRow->at(0) == primaryKey.get()) return index;
68+
if (bufferRow->at(0) == ID_GET(primaryKey)) return index;
6969
index++;
7070
}
7171
return BufferRowIndex();
@@ -79,7 +79,7 @@ BufferRowIndex NormalTable::getBufferIndexForPrimaryKey(ValidItemID primaryKey)
7979
*/
8080
ValidItemID NormalTable::getPrimaryKeyAt(BufferRowIndex bufferRowIndex) const
8181
{
82-
return buffer.getCell(bufferRowIndex, primaryKeyColumn->getIndex());
82+
return VALID_ITEM_ID(buffer.getCell(bufferRowIndex, primaryKeyColumn->getIndex()));
8383
}
8484

8585
/**
@@ -94,7 +94,7 @@ QList<QPair<ValidItemID, QVariant>> NormalTable::pairIDWith(const Column* column
9494
int columnIndex = column->getIndex();
9595
QList<QPair<ValidItemID, QVariant>> pairs = QList<QPair<ValidItemID, QVariant>>();
9696
for (const QList<QVariant>* const bufferRow : buffer) {
97-
pairs.append({ValidItemID(bufferRow->at(primaryKeyColumnIndex)), bufferRow->at(columnIndex)});
97+
pairs.append({VALID_ITEM_ID(bufferRow->at(primaryKeyColumnIndex)), bufferRow->at(columnIndex)});
9898
}
9999
return pairs;
100100
}

‎src/db/settings_table.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class SettingsTable : public Table {
6565
{
6666
assert(rowIndex < 2);
6767

68-
ValidItemID primaryKey = primaryKeyColumn->getValueAt(BufferRowIndex(rowIndex));
68+
ValidItemID primaryKey = VALID_ITEM_ID(primaryKeyColumn->getValueAt(BufferRowIndex(rowIndex)));
6969
updateCellInNormalTable(parent, primaryKey, setting, value);
7070
}
7171

‎src/db/table.cpp

+6-6
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ BufferRowIndex Table::getMatchingBufferRowIndex(const QList<const Column*>& prim
265265
for (BufferRowIndex bufferRowIndex = BufferRowIndex(0); bufferRowIndex.isValid(buffer.numRows()); bufferRowIndex++) {
266266
bool match = true;
267267
for (int i = 0; i < numPrimaryKeys; i++) {
268-
if (primaryKeyColumns.at(i)->getValueAt(bufferRowIndex) != primaryKeys.at(i).get()) {
268+
if (primaryKeyColumns.at(i)->getValueAt(bufferRowIndex) != ID_GET(primaryKeys.at(i))) {
269269
match = false;
270270
break;
271271
}
@@ -614,7 +614,7 @@ ValidItemID Table::addRowToSql(QWidget* parent, const QList<ColumnDataPair>& col
614614
if (!query.exec())
615615
displayError(parent, query.lastError(), queryString);
616616

617-
ValidItemID newRowID = query.lastInsertId().toInt();
617+
ValidItemID newRowID = VALID_ITEM_ID(query.lastInsertId().toInt());
618618
return newRowID;
619619
}
620620

@@ -636,7 +636,7 @@ void Table::updateCellOfNormalTableInSql(QWidget* parent, const ValidItemID prim
636636
QString queryString = QString(
637637
"UPDATE " + name +
638638
"\nSET " + column->name + " = ?" +
639-
"\nWHERE " + primaryKeyColumn->name + " = " + QString::number(primaryKey.get())
639+
"\nWHERE " + primaryKeyColumn->name + " = " + QString::number(ID_GET(primaryKey))
640640
);
641641
QSqlQuery query = QSqlQuery();
642642
if (!query.prepare(queryString))
@@ -667,7 +667,7 @@ void Table::updateRowInSql(QWidget* parent, const ValidItemID primaryKey, const
667667
QString queryString = QString(
668668
"UPDATE " + name +
669669
"\nSET " + setString +
670-
"\nWHERE " + primaryKeyColumn->name + " = " + QString::number(primaryKey.get())
670+
"\nWHERE " + primaryKeyColumn->name + " = " + QString::number(ID_GET(primaryKey))
671671
);
672672
QSqlQuery query = QSqlQuery();
673673
if (!query.prepare(queryString))
@@ -695,7 +695,7 @@ void Table::removeRowFromSql(QWidget* parent, const QList<const Column*>& primar
695695
const Column* column = primaryKeyColumns.at(i);
696696
assert(column->table == this && column->isPrimaryKey());
697697

698-
condition.append(column->name + " = " + QString::number(primaryKeys.at(i).get()));
698+
condition.append(column->name + " = " + QString::number(ID_GET(primaryKeys.at(i))));
699699
}
700700
QString queryString = QString(
701701
"DELETE FROM " + name +
@@ -721,7 +721,7 @@ void Table::removeMatchingRowsFromSql(QWidget* parent, const Column* column, Val
721721

722722
QString queryString = QString(
723723
"DELETE FROM " + name +
724-
"\nWHERE " + column->name + " = " + QString::number(key.get())
724+
"\nWHERE " + column->name + " = " + QString::number(ID_GET(key))
725725
);
726726
QSqlQuery query = QSqlQuery();
727727
query.setForwardOnly(true);

‎src/db/tables/ascents_table.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ void AscentsTable::updateRow(QWidget* parent, const Ascent* ascent)
9797
QList<const Column*> columns = getNonPrimaryKeyColumnList();
9898
const QList<ColumnDataPair> columnDataPairs = mapDataToColumnDataPairs(columns, ascent);
9999

100-
NormalTable::updateRow(parent, ascent->ascentID.forceValid(), columnDataPairs);
100+
NormalTable::updateRow(parent, FORCE_VALID(ascent->ascentID), columnDataPairs);
101101
}
102102

103103

0 commit comments

Comments
 (0)