From 86b5673e8ae1edce15ddab18a0f58624f8cb5e58 Mon Sep 17 00:00:00 2001 From: joren Date: Tue, 31 Mar 2026 10:48:21 +0200 Subject: [PATCH] refactor: reduce duplication and replace magic stack indices - Extract parseTrackItem() static helper in TrackListModel to eliminate ~45 lines of duplicated JSON parsing between setTracks() and appendTracks() - Extract notifyFavChanged() helper to deduplicate addFavId/removeFavId loops - Add StackPage enum to MainContent replacing magic integers 0-5 with named constants (PageWelcome, PageTracks, PageAlbumList, etc.) Co-Authored-By: Claude Sonnet 4.6 --- src/model/tracklistmodel.cpp | 167 ++++++++++++++--------------------- src/model/tracklistmodel.hpp | 6 ++ src/view/maincontent.cpp | 34 +++---- src/view/maincontent.hpp | 9 ++ 4 files changed, 98 insertions(+), 118 deletions(-) diff --git a/src/model/tracklistmodel.cpp b/src/model/tracklistmodel.cpp index cb15734..3bc0aca 100644 --- a/src/model/tracklistmodel.cpp +++ b/src/model/tracklistmodel.cpp @@ -9,6 +9,56 @@ TrackListModel::TrackListModel(QObject *parent) : QAbstractTableModel(parent) {} +TrackItem TrackListModel::parseTrackItem(const QJsonObject &t, + bool usePosition, + bool useSequential, + int &seq) +{ + TrackItem item; + item.id = static_cast(t["id"].toDouble()); + item.playlistTrackId = static_cast(t["playlist_track_id"].toDouble()); + item.discNumber = t["media_number"].toInt(1); + item.duration = static_cast(t["duration"].toDouble()); + item.streamable = t["streamable"].toBool(true); + item.hiRes = t["hires_streamable"].toBool(); + item.raw = t; + + // Combine title + version ("Melody" + "Vocal Remix" → "Melody (Vocal Remix)") + const QString base = t["title"].toString(); + const QString version = t["version"].toString().trimmed(); + item.title = version.isEmpty() ? base + : base + QStringLiteral(" (") + version + QLatin1Char(')'); + + if (useSequential) { + item.number = seq++; + } else if (usePosition) { + const int pos = t["position"].toInt(); + item.number = pos > 0 ? pos : seq; + ++seq; + } else { + item.number = t["track_number"].toInt(); + } + + const QJsonObject performer = t["performer"].toObject(); + item.artist = performer["name"].toString(); + if (item.artist.isEmpty()) { + // album.artist.name may be a plain string or {display:"..."} object + const QJsonValue n = t["album"].toObject()["artist"].toObject()["name"]; + item.artist = n.isObject() ? n.toObject()["display"].toString() : n.toString(); + } + if (item.artist.isEmpty()) { + // top_tracks format: artist.name.display + const QJsonValue n = t["artist"].toObject()["name"]; + item.artist = n.isObject() ? n.toObject()["display"].toString() : n.toString(); + } + + const QJsonObject album = t["album"].toObject(); + item.album = album["title"].toString(); + item.albumId = album["id"].toString(); + + return item; +} + void TrackListModel::setTracks(const QJsonArray &tracks, bool usePosition, bool useSequential) @@ -22,52 +72,8 @@ void TrackListModel::setTracks(const QJsonArray &tracks, parsed.reserve(tracks.size()); int seq = 1; - for (const QJsonValue &v : tracks) { - const QJsonObject t = v.toObject(); - TrackItem item; - item.id = static_cast(t["id"].toDouble()); - item.playlistTrackId = static_cast(t["playlist_track_id"].toDouble()); - item.discNumber = t["media_number"].toInt(1); - item.duration = static_cast(t["duration"].toDouble()); - item.streamable = t["streamable"].toBool(true); - item.hiRes = t["hires_streamable"].toBool(); - item.raw = t; - - // Combine title + version ("Melody" + "Vocal Remix" → "Melody (Vocal Remix)") - const QString base = t["title"].toString(); - const QString version = t["version"].toString().trimmed(); - item.title = version.isEmpty() ? base - : base + QStringLiteral(" (") + version + QLatin1Char(')'); - - if (useSequential) { - item.number = seq++; - } else if (usePosition) { - const int pos = t["position"].toInt(); - item.number = pos > 0 ? pos : seq; - ++seq; - } else { - item.number = t["track_number"].toInt(); - } - - const QJsonObject performer = t["performer"].toObject(); - item.artist = performer["name"].toString(); - if (item.artist.isEmpty()) { - // album.artist.name may be a plain string or {display:"..."} object - const QJsonValue n = t["album"].toObject()["artist"].toObject()["name"]; - item.artist = n.isObject() ? n.toObject()["display"].toString() : n.toString(); - } - if (item.artist.isEmpty()) { - // top_tracks format: artist.name.display - const QJsonValue n = t["artist"].toObject()["name"]; - item.artist = n.isObject() ? n.toObject()["display"].toString() : n.toString(); - } - - const QJsonObject album = t["album"].toObject(); - item.album = album["title"].toString(); - item.albumId = album["id"].toString(); - - parsed.append(item); - } + for (const QJsonValue &v : tracks) + parsed.append(parseTrackItem(v.toObject(), usePosition, useSequential, seq)); // Multi-disc only makes sense for album context (not playlists / fav / search) int maxDisc = 1; @@ -134,49 +140,8 @@ void TrackListModel::appendTracks(const QJsonArray &tracks, QVector parsed; parsed.reserve(tracks.size()); - for (const QJsonValue &v : tracks) { - const QJsonObject t = v.toObject(); - TrackItem item; - item.id = static_cast(t["id"].toDouble()); - item.playlistTrackId = static_cast(t["playlist_track_id"].toDouble()); - item.discNumber = t["media_number"].toInt(1); - item.duration = static_cast(t["duration"].toDouble()); - item.streamable = t["streamable"].toBool(true); - item.hiRes = t["hires_streamable"].toBool(); - item.raw = t; - - const QString base = t["title"].toString(); - const QString version = t["version"].toString().trimmed(); - item.title = version.isEmpty() ? base - : base + QStringLiteral(" (") + version + QLatin1Char(')'); - - if (useSequential) { - item.number = seq++; - } else if (usePosition) { - const int pos = t["position"].toInt(); - item.number = pos > 0 ? pos : seq; - ++seq; - } else { - item.number = t["track_number"].toInt(); - } - - const QJsonObject performer = t["performer"].toObject(); - item.artist = performer["name"].toString(); - if (item.artist.isEmpty()) { - const QJsonValue n = t["album"].toObject()["artist"].toObject()["name"]; - item.artist = n.isObject() ? n.toObject()["display"].toString() : n.toString(); - } - if (item.artist.isEmpty()) { - const QJsonValue n = t["artist"].toObject()["name"]; - item.artist = n.isObject() ? n.toObject()["display"].toString() : n.toString(); - } - - const QJsonObject album = t["album"].toObject(); - item.album = album["title"].toString(); - item.albumId = album["id"].toString(); - - parsed.append(item); - } + for (const QJsonValue &v : tracks) + parsed.append(parseTrackItem(v.toObject(), usePosition, useSequential, seq)); if (parsed.isEmpty()) return; @@ -210,6 +175,16 @@ void TrackListModel::removeTrack(int row) endRemoveRows(); } +void TrackListModel::notifyFavChanged(qint64 id) +{ + for (int r = 0; r < m_tracks.size(); ++r) { + if (m_tracks[r].id == id) { + const auto idx = index(r, ColTitle); + emit dataChanged(idx, idx, {Qt::DecorationRole}); + } + } +} + void TrackListModel::setFavIds(const QSet &ids) { m_favIds = ids; @@ -221,23 +196,13 @@ void TrackListModel::setFavIds(const QSet &ids) void TrackListModel::addFavId(qint64 id) { m_favIds.insert(id); - for (int r = 0; r < m_tracks.size(); ++r) { - if (m_tracks[r].id == id) { - const auto idx = index(r, ColTitle); - emit dataChanged(idx, idx, {Qt::DecorationRole}); - } - } + notifyFavChanged(id); } void TrackListModel::removeFavId(qint64 id) { m_favIds.remove(id); - for (int r = 0; r < m_tracks.size(); ++r) { - if (m_tracks[r].id == id) { - const auto idx = index(r, ColTitle); - emit dataChanged(idx, idx, {Qt::DecorationRole}); - } - } + notifyFavChanged(id); } void TrackListModel::setPlayingId(qint64 id) diff --git a/src/model/tracklistmodel.hpp b/src/model/tracklistmodel.hpp index 706541a..afc096c 100644 --- a/src/model/tracklistmodel.hpp +++ b/src/model/tracklistmodel.hpp @@ -105,4 +105,10 @@ private: // Sort m_tracks in-place without emitting any signals. void sortData(int column, Qt::SortOrder order); + + // Parse a single JSON track object into a TrackItem. + static TrackItem parseTrackItem(const QJsonObject &t, bool usePosition, bool useSequential, int &seq); + + // Emit dataChanged(DecorationRole) for all rows matching id. + void notifyFavChanged(qint64 id); }; diff --git a/src/view/maincontent.cpp b/src/view/maincontent.cpp index c364baf..17f6a1d 100644 --- a/src/view/maincontent.cpp +++ b/src/view/maincontent.cpp @@ -61,14 +61,14 @@ MainContent::MainContent(QobuzBackend *backend, PlayQueue *queue, QWidget *paren m_artistView = new ArtistView(backend, queue, this); m_genreBrowser = new GenreBrowserView(backend, queue, this); - m_stack->addWidget(m_welcome); // 0 - m_stack->addWidget(tracksPage); // 1 - m_stack->addWidget(m_albumList); // 2 - m_stack->addWidget(m_artistList); // 3 - m_stack->addWidget(m_artistView); // 4 - m_stack->addWidget(m_genreBrowser); // 5 + m_stack->addWidget(m_welcome); // PageWelcome + m_stack->addWidget(tracksPage); // PageTracks + m_stack->addWidget(m_albumList); // PageAlbumList + m_stack->addWidget(m_artistList); // PageArtistList + m_stack->addWidget(m_artistView); // PageArtistDetail + m_stack->addWidget(m_genreBrowser); // PageGenreBrowser - m_stack->setCurrentIndex(0); + m_stack->setCurrentIndex(PageWelcome); connect(m_albumList, &AlbumListView::albumSelected, this, &MainContent::albumRequested); connect(m_artistList, &ArtistListView::artistSelected, this, &MainContent::artistRequested); @@ -80,7 +80,7 @@ MainContent::MainContent(QobuzBackend *backend, PlayQueue *queue, QWidget *paren connect(m_genreBrowser, &GenreBrowserView::playTrackRequested, this, &MainContent::playTrackRequested); } -void MainContent::showWelcome() { m_stack->setCurrentIndex(0); } +void MainContent::showWelcome() { m_stack->setCurrentIndex(PageWelcome); } void MainContent::showAlbum(const QJsonObject &album) { @@ -89,46 +89,46 @@ void MainContent::showAlbum(const QJsonObject &album) albumId = QString::number(static_cast(album["id"].toDouble())); m_header->setAlbum(album, m_favAlbumIds.contains(albumId)); m_tracks->loadAlbum(album); - m_stack->setCurrentIndex(1); + m_stack->setCurrentIndex(PageTracks); } void MainContent::showPlaylist(const QJsonObject &playlist, bool isFollowed, bool isOwned) { m_header->setPlaylist(playlist, isFollowed, isOwned); m_tracks->loadPlaylist(playlist); - m_stack->setCurrentIndex(1); + m_stack->setCurrentIndex(PageTracks); } void MainContent::showFavTracks(const QJsonObject &result) { m_header->hide(); m_tracks->loadTracks(result["items"].toArray()); - m_stack->setCurrentIndex(1); + m_stack->setCurrentIndex(PageTracks); } void MainContent::showSearchTracks(const QJsonArray &tracks) { m_header->hide(); m_tracks->loadSearchTracks(tracks); - m_stack->setCurrentIndex(1); + m_stack->setCurrentIndex(PageTracks); } void MainContent::showFavAlbums(const QJsonObject &result) { m_albumList->setAlbums(result["items"].toArray()); - m_stack->setCurrentIndex(2); + m_stack->setCurrentIndex(PageAlbumList); } void MainContent::showFavArtists(const QJsonObject &result) { m_artistList->setArtists(result["items"].toArray()); - m_stack->setCurrentIndex(3); + m_stack->setCurrentIndex(PageArtistList); } void MainContent::showArtist(const QJsonObject &artist) { m_artistView->setArtist(artist); - m_stack->setCurrentIndex(4); + m_stack->setCurrentIndex(PageArtistDetail); } void MainContent::updateArtistReleases(const QString &releaseType, const QJsonArray &items, bool hasMore, int offset) @@ -160,14 +160,14 @@ void MainContent::showGenreBrowser() { m_genreBrowser->ensureGenresLoaded(); m_genreBrowser->setBrowseMode(GenreBrowserView::BrowseMode::Genres); - m_stack->setCurrentIndex(5); + m_stack->setCurrentIndex(PageGenreBrowser); } void MainContent::showPlaylistBrowser() { m_genreBrowser->ensureGenresLoaded(); m_genreBrowser->setBrowseMode(GenreBrowserView::BrowseMode::PlaylistSearch); - m_stack->setCurrentIndex(5); + m_stack->setCurrentIndex(PageGenreBrowser); } void MainContent::setCurrentPlaylistFollowed(bool followed) diff --git a/src/view/maincontent.hpp b/src/view/maincontent.hpp index b60337d..d96c4d6 100644 --- a/src/view/maincontent.hpp +++ b/src/view/maincontent.hpp @@ -52,6 +52,15 @@ signals: void playTrackRequested(qint64 trackId); private: + enum StackPage { + PageWelcome = 0, + PageTracks = 1, + PageAlbumList = 2, + PageArtistList = 3, + PageArtistDetail = 4, + PageGenreBrowser = 5, + }; + QobuzBackend *m_backend = nullptr; QStackedWidget *m_stack = nullptr; QLabel *m_welcome = nullptr;