From 082f84ace487290b02eb9e6e9ceb0c162a63fd4f Mon Sep 17 00:00:00 2001 From: m0dB Date: Sat, 14 Dec 2024 18:37:14 +0100 Subject: [PATCH] reduced nested by using early returns, renamed things for clarity --- src/library/basetrackcache.cpp | 40 ++++++++++++++--------------- src/library/basetracktablemodel.cpp | 2 +- src/track/keyutils.cpp | 24 ++++++++++------- src/track/keyutils.h | 5 +++- 4 files changed, 40 insertions(+), 31 deletions(-) diff --git a/src/library/basetrackcache.cpp b/src/library/basetrackcache.cpp index 286cea9a290..4167faa9069 100644 --- a/src/library/basetrackcache.cpp +++ b/src/library/basetrackcache.cpp @@ -401,7 +401,8 @@ QVariant BaseTrackCache::getTrackValueForColumn(TrackPointer pTrack, } if (fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_KEY) == column) { // The Key value is determined by either the KEY_ID or KEY column - return KeyUtils::keyFromColumns(QVariant{pTrack->getKeyText()}, QVariant{pTrack->getKey()}); + return QVariant{KeyUtils::keyFromKeyTextAndIdValues( + pTrack->getKeyText(), pTrack->getKey())}; } if (fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_KEY_ID) == column) { return QVariant{static_cast(pTrack->getKey())}; @@ -437,17 +438,18 @@ QVariant BaseTrackCache::getTrackValueForColumn(TrackPointer pTrack, } QVariant BaseTrackCache::data(TrackId trackId, int column) const { - QVariant result; - if (!m_bIndexBuilt) { qDebug() << this << "ERROR index is not built for" << m_tableName; - return result; + return QVariant{}; } if (m_bIsCaching) { TrackPointer pTrack = getRecentTrack(trackId); if (pTrack) { - result = getTrackValueForColumn(pTrack, column); + QVariant result = getTrackValueForColumn(pTrack, column); + if (result.isValid()) { + return result; + } } } @@ -458,22 +460,20 @@ QVariant BaseTrackCache::data(TrackId trackId, int column) const { // TODO(rryan) this code is flawed for columns that contains row-specific // metadata. Currently the upper-levels will not delegate row-specific // columns to this method, but there should still be a check here I think. - if (!result.isValid()) { - auto it = m_trackInfo.constFind(trackId); - if (it != m_trackInfo.constEnd()) { - const QVector& fields = it.value(); - if (column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_KEY)) { - // The Key value is determined by either the KEY_ID or KEY column - const auto columnForKeyId = fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_KEY_ID); - result = KeyUtils::keyFromColumns( - fields.value(column, result), - fields.value(columnForKeyId, result)); - } else { - result = fields.value(column, result); - } - } + auto it = m_trackInfo.constFind(trackId); + if (it == m_trackInfo.constEnd()) { + return QVariant{}; } - return result; + + const QVector& fields = it.value(); + if (column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_KEY)) { + // The Key value is determined by either the KEY_ID or KEY column + const auto columnForKeyId = fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_KEY_ID); + return KeyUtils::keyFromKeyTextAndIdFields( + fields.value(column, QVariant{}), + fields.value(columnForKeyId, QVariant{})); + } + return fields.value(column, QVariant{}); } void BaseTrackCache::filterAndSort(const QSet& trackIds, diff --git a/src/library/basetracktablemodel.cpp b/src/library/basetracktablemodel.cpp index bb423ed0a22..8ce9c7a65a1 100644 --- a/src/library/basetracktablemodel.cpp +++ b/src/library/basetracktablemodel.cpp @@ -874,7 +874,7 @@ QVariant BaseTrackTableModel::roleValue( } case ColumnCache::COLUMN_LIBRARYTABLE_KEY: // The Key value is determined by either the KEY_ID or KEY column - return KeyUtils::keyFromColumns( + return KeyUtils::keyFromKeyTextAndIdFields( rawValue, rawSiblingValue( index, ColumnCache::COLUMN_LIBRARYTABLE_KEY_ID)); diff --git a/src/track/keyutils.cpp b/src/track/keyutils.cpp index 83bee49bda1..017316bc150 100644 --- a/src/track/keyutils.cpp +++ b/src/track/keyutils.cpp @@ -769,7 +769,8 @@ int KeyUtils::keyToCircleOfFifthsOrder(mixxx::track::io::key::ChromaticKey key, } // static -QVariant KeyUtils::keyFromColumns(const QVariant& keyValue, const QVariant& keyIdValue) { +QVariant KeyUtils::keyFromKeyTextAndIdFields( + const QVariant& keyTextField, const QVariant& keyIdField) { // Helper function used by basetrackcache.cpp and basetracktablemodel.cpp // to determine the Key string from either the LIBRARYTABLE_KEY or the // LIBRARYTABLE_KEY_ID field. @@ -778,23 +779,28 @@ QVariant KeyUtils::keyFromColumns(const QVariant& keyValue, const QVariant& keyI // column (as opposed to the string representation of the key // currently stored in the DB) then lookup the key and render it // using the user's selected notation. - if (keyIdValue.isNull()) { + if (keyIdField.isNull()) { // Otherwise, just use the KEY column value as is - return keyValue; + return keyTextField; } // Convert or clear invalid values - VERIFY_OR_DEBUG_ASSERT(keyIdValue.canConvert()) { - return QVariant(); + VERIFY_OR_DEBUG_ASSERT(keyIdField.canConvert()) { + return keyTextField; } bool ok; - const auto keyId = keyIdValue.toInt(&ok); + const auto keyId = keyIdField.toInt(&ok); VERIFY_OR_DEBUG_ASSERT(ok) { - return QVariant(); + return keyTextField; } const auto key = KeyUtils::keyFromNumericValue(keyId); if (key == mixxx::track::io::key::INVALID) { - return QVariant(); + return keyTextField; } // Render the key with the user-provided notation - return KeyUtils::keyToString(key); + return QVariant{KeyUtils::keyToString(key)}; +} + +// static +QString KeyUtils::keyFromKeyTextAndIdValues(const QString& keyText, const ChromaticKey& keyId) { + return keyId == mixxx::track::io::key::INVALID ? keyText : KeyUtils::keyToString(keyId); } diff --git a/src/track/keyutils.h b/src/track/keyutils.h index 5c3065e16a7..51e3e619bfb 100644 --- a/src/track/keyutils.h +++ b/src/track/keyutils.h @@ -132,7 +132,10 @@ class KeyUtils { static int keyToCircleOfFifthsOrder(mixxx::track::io::key::ChromaticKey key, KeyNotation notation); - static QVariant keyFromColumns(const QVariant& rawValue, const QVariant& keyCodeValue); + static QVariant keyFromKeyTextAndIdFields( + const QVariant& keyTextField, const QVariant& keyIdField); + static QString keyFromKeyTextAndIdValues(const QString& keyText, + const mixxx::track::io::key::ChromaticKey& key); private: static QMutex s_notationMutex;