diff options
author | Uwe Klotz <uklotz@mixxx.org> | 2020-11-10 12:48:25 +0100 |
---|---|---|
committer | Uwe Klotz <uklotz@mixxx.org> | 2020-11-15 14:34:14 +0100 |
commit | c9170a8e0b860a3906005c677e4674cbcc74c32a (patch) | |
tree | 6641189f6530118a977fa5215f67147055f4e3e9 | |
parent | 3e1f1c44db104ccc806779802c00eb22cc35c74b (diff) |
Cue/CueDAO: Improve type-safety and reduce code duplication
-rw-r--r-- | src/dialog/dlgreplacecuecolor.cpp | 8 | ||||
-rw-r--r-- | src/library/dao/cuedao.cpp | 98 | ||||
-rw-r--r-- | src/track/cue.cpp | 19 | ||||
-rw-r--r-- | src/track/cue.h | 13 | ||||
-rw-r--r-- | src/track/track.cpp | 2 | ||||
-rw-r--r-- | src/track/track.h | 2 |
6 files changed, 70 insertions, 72 deletions
diff --git a/src/dialog/dlgreplacecuecolor.cpp b/src/dialog/dlgreplacecuecolor.cpp index 2220524559..0a68619bc5 100644 --- a/src/dialog/dlgreplacecuecolor.cpp +++ b/src/dialog/dlgreplacecuecolor.cpp @@ -31,7 +31,7 @@ void setButtonColor(QPushButton* button, const QColor& color) { } typedef struct { - int id; + DbId id; TrackId trackId; mixxx::RgbColor color; } CueDatabaseRow; @@ -339,7 +339,7 @@ void DlgReplaceCueColor::slotApply() { VERIFY_OR_DEBUG_ASSERT(color) { continue; } - CueDatabaseRow row = {selectQuery.value(idColumn).toInt(), + CueDatabaseRow row = {DbId(selectQuery.value(idColumn)), TrackId(selectQuery.value(trackIdColumn).toInt()), *color}; rows << row; @@ -383,14 +383,14 @@ void DlgReplaceCueColor::slotApply() { bool canceled = false; - QMultiMap<TrackPointer, int> cues; + QMultiMap<TrackPointer, DbId> cues; for (const auto& row : qAsConst(rows)) { QCoreApplication::processEvents(); if (progress.wasCanceled()) { canceled = true; break; } - query.bindValue(":id", row.id); + query.bindValue(":id", row.id.toVariant()); query.bindValue(":track_id", row.trackId.value()); query.bindValue(":current_color", mixxx::RgbColor::toQVariant(row.color)); if (!query.exec()) { diff --git a/src/library/dao/cuedao.cpp b/src/library/dao/cuedao.cpp index fb76e915c6..a65d08711f 100644 --- a/src/library/dao/cuedao.cpp +++ b/src/library/dao/cuedao.cpp @@ -40,7 +40,7 @@ inline QString labelFromQVariant(const QVariant& value) { } CuePointer cueFromRow(const QSqlRecord& row) { - int id = row.value(row.indexOf("id")).toInt(); + const auto id = DbId(row.value(row.indexOf("id"))); TrackId trackId(row.value(row.indexOf("track_id"))); int type = row.value(row.indexOf("type")).toInt(); int position = row.value(row.indexOf("position")).toInt(); @@ -142,28 +142,11 @@ bool CueDAO::saveCue(Cue* cue) const { VERIFY_OR_DEBUG_ASSERT(cue) { return false; } - if (cue->getId() == -1) { - // New cue - QSqlQuery query(m_database); - query.prepare(QStringLiteral("INSERT INTO " CUE_TABLE " (track_id, type, position, length, hotcue, label, color) VALUES (:track_id, :type, :position, :length, :hotcue, :label, :color)")); - query.bindValue(":track_id", cue->getTrackId().toVariant()); - query.bindValue(":type", static_cast<int>(cue->getType())); - query.bindValue(":position", cue->getPosition()); - query.bindValue(":length", cue->getLength()); - query.bindValue(":hotcue", cue->getHotCue()); - query.bindValue(":label", labelToQVariant(cue->getLabel())); - query.bindValue(":color", mixxx::RgbColor::toQVariant(cue->getColor())); - if (query.exec()) { - int id = query.lastInsertId().toInt(); - cue->setId(id); - cue->setDirty(false); - return true; - } - qDebug() << query.executedQuery() << query.lastError(); - } else { + // Prepare query + QSqlQuery query(m_database); + if (cue->getId().isValid()) { // Update cue - QSqlQuery query(m_database); query.prepare(QStringLiteral("UPDATE " CUE_TABLE " SET " "track_id=:track_id," "type=:type," @@ -173,40 +156,53 @@ bool CueDAO::saveCue(Cue* cue) const { "label=:label," "color=:color" " WHERE id=:id")); - query.bindValue(":id", cue->getId()); - query.bindValue(":track_id", cue->getTrackId().toVariant()); - query.bindValue(":type", static_cast<int>(cue->getType())); - query.bindValue(":position", cue->getPosition()); - query.bindValue(":length", cue->getLength()); - query.bindValue(":hotcue", cue->getHotCue()); - query.bindValue(":label", labelToQVariant(cue->getLabel())); - query.bindValue(":color", mixxx::RgbColor::toQVariant(cue->getColor())); + query.bindValue(":id", cue->getId().toVariant()); + } else { + // New cue + query.prepare( + QStringLiteral("INSERT INTO " CUE_TABLE + " (track_id, type, position, length, hotcue, " + "label, color) VALUES (:track_id, :type, " + ":position, :length, :hotcue, :label, :color)")); + } - if (query.exec()) { - cue->setDirty(false); - return true; - } else { - LOG_FAILED_QUERY(query); - } + // Bind values and execute query + query.bindValue(":track_id", cue->getTrackId().toVariant()); + query.bindValue(":type", static_cast<int>(cue->getType())); + query.bindValue(":position", cue->getPosition()); + query.bindValue(":length", cue->getLength()); + query.bindValue(":hotcue", cue->getHotCue()); + query.bindValue(":label", labelToQVariant(cue->getLabel())); + query.bindValue(":color", mixxx::RgbColor::toQVariant(cue->getColor())); + if (!query.exec()) { + LOG_FAILED_QUERY(query); + return false; } - return false; + + if (!cue->getId().isValid()) { + // New cue + const auto newId = DbId(query.lastInsertId()); + DEBUG_ASSERT(newId.isValid()); + cue->setId(newId); + } + DEBUG_ASSERT(cue->getId().isValid()); + cue->setDirty(false); + return true; } bool CueDAO::deleteCue(Cue* cue) const { //qDebug() << "CueDAO::deleteCue" << QThread::currentThread() << m_database.connectionName(); - if (cue->getId() != -1) { - QSqlQuery query(m_database); - query.prepare(QStringLiteral("DELETE FROM " CUE_TABLE " WHERE id=:id")); - query.bindValue(":id", cue->getId()); - if (query.exec()) { - return true; - } else { - LOG_FAILED_QUERY(query); - } - } else { - return true; + if (!cue->getId().isValid()) { + return false; } - return false; + QSqlQuery query(m_database); + query.prepare(QStringLiteral("DELETE FROM " CUE_TABLE " WHERE id=:id")); + query.bindValue(":id", cue->getId().toVariant()); + if (!query.exec()) { + LOG_FAILED_QUERY(query); + return false; + } + return true; } void CueDAO::saveTrackCues( @@ -219,16 +215,16 @@ void CueDAO::saveTrackCues( pCue->setTrackId(trackId); } // New cues (without an id) must always be marked as dirty - DEBUG_ASSERT(pCue->getId() >= 0 || pCue->isDirty()); + DEBUG_ASSERT(pCue->getId().isValid() || pCue->isDirty()); // Update or save cue if (pCue->isDirty()) { saveCue(pCue.get()); } // After saving each cue must have a valid id - VERIFY_OR_DEBUG_ASSERT(pCue->getId() >= 0) { + VERIFY_OR_DEBUG_ASSERT(pCue->getId().isValid()) { continue; } - cueIds.append(QString::number(pCue->getId())); + cueIds.append(pCue->getId().toString()); } // Delete orphaned cues diff --git a/src/track/cue.cpp b/src/track/cue.cpp index 4cc27aa206..f431513138 100644 --- a/src/track/cue.cpp +++ b/src/track/cue.cpp @@ -49,16 +49,16 @@ void CuePointer::deleteLater(Cue* pCue) { Cue::Cue() : m_bDirty(false), - m_iId(-1), m_type(mixxx::CueType::Invalid), m_sampleStartPosition(Cue::kNoPosition), m_sampleEndPosition(Cue::kNoPosition), m_iHotCue(Cue::kNoHotCue), m_color(mixxx::PredefinedColorPalettes::kDefaultCueColor) { + DEBUG_ASSERT(!m_dbId.isValid()); } Cue::Cue( - int id, + DbId id, TrackId trackId, mixxx::CueType type, double position, @@ -66,14 +66,15 @@ Cue::Cue( int hotCue, QString label, mixxx::RgbColor color) - : m_bDirty(false), - m_iId(id), + : m_bDirty(false), // clear flag after loading from database + m_dbId(id), m_trackId(trackId), m_type(type), m_sampleStartPosition(position), m_iHotCue(hotCue), m_label(label), m_color(color) { + DEBUG_ASSERT(m_dbId.isValid()); if (length != 0) { if (position != Cue::kNoPosition) { m_sampleEndPosition = position + length; @@ -90,7 +91,6 @@ Cue::Cue( mixxx::audio::SampleRate sampleRate, bool setDirty) : m_bDirty(setDirty), - m_iId(-1), m_type(cueInfo.getType()), m_sampleStartPosition( positionMillisToSamples( @@ -103,6 +103,7 @@ Cue::Cue( m_iHotCue(cueInfo.getHotCueNumber() ? *cueInfo.getHotCueNumber() : kNoHotCue), m_label(cueInfo.getLabel()), m_color(cueInfo.getColor().value_or(mixxx::PredefinedColorPalettes::kDefaultCueColor)) { + DEBUG_ASSERT(!m_dbId.isValid()); } mixxx::CueInfo Cue::getCueInfo( @@ -117,14 +118,14 @@ mixxx::CueInfo Cue::getCueInfo( m_color); } -int Cue::getId() const { +DbId Cue::getId() const { QMutexLocker lock(&m_mutex); - return m_iId; + return m_dbId; } -void Cue::setId(int cueId) { +void Cue::setId(DbId cueId) { QMutexLocker lock(&m_mutex); - m_iId = cueId; + m_dbId = cueId; // Neither mark as dirty nor do emit the updated() signal. // This function is only called after adding the Cue object // to the database. The id is not visible for anyone else. diff --git a/src/track/cue.h b/src/track/cue.h index 7ea735d89c..3a3f05197f 100644 --- a/src/track/cue.h +++ b/src/track/cue.h @@ -3,14 +3,14 @@ #include <QColor> #include <QMutex> #include <QObject> +#include <memory> #include "audio/types.h" #include "track/cueinfo.h" #include "track/trackid.h" #include "util/color/rgbcolor.h" -#include "util/memory.h" +#include "util/db/dbid.h" -class CuePosition; class CueDAO; class Track; @@ -26,8 +26,9 @@ class Cue : public QObject { const mixxx::CueInfo& cueInfo, mixxx::audio::SampleRate sampleRate, bool setDirty); + /// Load entity from database. Cue( - int id, + DbId id, TrackId trackId, mixxx::CueType type, double position, @@ -38,7 +39,7 @@ class Cue : public QObject { ~Cue() override = default; bool isDirty() const; - int getId() const; + DbId getId() const; TrackId getTrackId() const; mixxx::CueType getType() const; @@ -75,13 +76,13 @@ class Cue : public QObject { private: void setDirty(bool dirty); - void setId(int id); + void setId(DbId dbId); void setTrackId(TrackId trackId); mutable QMutex m_mutex; bool m_bDirty; - int m_iId; + DbId m_dbId; TrackId m_trackId; mixxx::CueType m_type; double m_sampleStartPosition; diff --git a/src/track/track.cpp b/src/track/track.cpp index a02810343b..75e4fa3c29 100644 --- a/src/track/track.cpp +++ b/src/track/track.cpp @@ -853,7 +853,7 @@ CuePointer Track::findCueByType(mixxx::CueType type) const { return CuePointer(); } -CuePointer Track::findCueById(int id) const { +CuePointer Track::findCueById(DbId id) const { QMutexLocker lock(&m_qMutex); for (const CuePointer& pCue : m_cuePoints) { if (pCue->getId() == id) { diff --git a/src/track/track.h b/src/track/track.h index 6cf9905d34..802b298c43 100644 --- a/src/track/track.h +++ b/src/track/track.h @@ -250,7 +250,7 @@ class Track : public QObject { // Calls for managing the track's cue points CuePointer createAndAddCue(); CuePointer findCueByType(mixxx::CueType type) const; // NOTE: Cannot be used for hotcues. - CuePointer findCueById(int id) const; + CuePointer findCueById(DbId id) const; void removeCue(const CuePointer& pCue); void removeCuesOfType(mixxx::CueType); QList<CuePointer> getCuePoints() const { |