Skip to content

Commit

Permalink
reduced nested by using early returns, renamed things for clarity
Browse files Browse the repository at this point in the history
  • Loading branch information
m0dB authored and m0dB committed Dec 14, 2024
1 parent 90feb67 commit 082f84a
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 31 deletions.
40 changes: 20 additions & 20 deletions src/library/basetrackcache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>(pTrack->getKey())};
Expand Down Expand Up @@ -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;
}
}
}

Expand All @@ -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<QVariant>& 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<QVariant>& 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<TrackId>& trackIds,
Expand Down
2 changes: 1 addition & 1 deletion src/library/basetracktablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
24 changes: 15 additions & 9 deletions src/track/keyutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<int>()) {
return QVariant();
VERIFY_OR_DEBUG_ASSERT(keyIdField.canConvert<int>()) {
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);
}
5 changes: 4 additions & 1 deletion src/track/keyutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 082f84a

Please sign in to comment.