Improve dbquery code

dbquery/dbquery.h

@@ -110,8 +110,8 @@ inline bool QueryResultsModel::isFetchingCover() const
 class HttpResultsModel : public QueryResultsModel {
     Q_OBJECT
 public:
-    ~HttpResultsModel();
+    ~HttpResultsModel() override;
-    void abort();
+    void abort() override;
 
 protected:
     HttpResultsModel(SongDescription &&initialSongDescription, QNetworkReply *reply);

dbquery/lyricswikia.cpp

@@ -33,55 +33,72 @@ LyricsWikiaResultsModel::LyricsWikiaResultsModel(SongDescription &&initialSongDe
 
 bool LyricsWikiaResultsModel::fetchCover(const QModelIndex &index)
 {
+    // FIXME: avoid code duplication with musicbrainz.cpp
+
+    // find song description
     if (index.parent().isValid() || index.row() >= m_results.size()) {
         return true;
     }
     SongDescription &desc = m_results[index.row()];
+
+    // skip if cover is already available
     if (!desc.cover.isEmpty()) {
-        // cover is already available -> nothing to do
         return true;
     }
+
+    // fail if album ID is unknown
     if (desc.albumId.isEmpty()) {
         m_errorList << tr("Unable to fetch cover: Album ID unknown");
         emit resultsAvailable();
         return true;
     }
-    try {
+
-        // the item belongs to an album which cover has already been fetched
+    // skip if the item belongs to an album which cover has already been fetched
-        desc.cover = m_coverData.at(desc.albumId);
+    const auto coverData = m_coverData.find(desc.albumId);
-    } catch (const out_of_range &) {
+    if (coverData != m_coverData.end()) {
-        if (desc.coverUrl.isEmpty()) {
+        desc.cover = coverData->second;
-            // request the cover URL
+        return true;
-            auto *const reply = requestAlbumDetails(desc);
-            addReply(reply, bind(&LyricsWikiaResultsModel::handleAlbumDetailsReplyFinished, this, reply, index.row()));
-            setFetchingCover(true);
-        } else {
-            // request the cover art
-            auto *const reply = networkAccessManager().get(QNetworkRequest(QUrl(desc.coverUrl)));
-            addReply(reply, bind(&LyricsWikiaResultsModel::handleCoverReplyFinished, this, reply, desc.albumId, index.row()));
-            setFetchingCover(true);
-        }
-        return false;
     }
-    return true;
+
+    // start http request
+    if (desc.coverUrl.isEmpty()) {
+        // request the cover URL
+        auto *const reply = requestAlbumDetails(desc);
+        addReply(reply, bind(&LyricsWikiaResultsModel::handleAlbumDetailsReplyFinished, this, reply, index.row()));
+        setFetchingCover(true);
+    } else {
+        // request the cover art
+        auto *const reply = networkAccessManager().get(QNetworkRequest(QUrl(desc.coverUrl)));
+        addReply(reply, bind(&LyricsWikiaResultsModel::handleCoverReplyFinished, this, reply, desc.albumId, index.row()));
+        setFetchingCover(true);
+    }
+    return false;
 }
 
 bool LyricsWikiaResultsModel::fetchLyrics(const QModelIndex &index)
 {
-    if (!index.parent().isValid() && index.row() < m_results.size()) {
+    // find song description
-        SongDescription &desc = m_results[index.row()];
+    if (index.parent().isValid() || index.row() >= m_results.size()) {
-        if (!desc.lyrics.isEmpty()) {
+        return true;
-            // lyrics already available -> nothing to do
-        } else if (!desc.artist.isEmpty() && !desc.title.isEmpty()) {
-            auto *reply = requestSongDetails(desc);
-            addReply(reply, bind(&LyricsWikiaResultsModel::handleSongDetailsFinished, this, reply, index.row()));
-            return false;
-        } else {
-            m_errorList << tr("Unable to fetch lyrics: Artist or title is unknown.");
-            emit resultsAvailable();
-        }
     }
-    return true;
+    SongDescription &desc = m_results[index.row()];
+
+    // skip if lyrics already present
+    if (!desc.lyrics.isEmpty()) {
+        return true;
+    }
+
+    // fail if artist or title unknown
+    if (desc.artist.isEmpty() || desc.title.isEmpty()) {
+        m_errorList << tr("Unable to fetch lyrics: Artist or title is unknown.");
+        emit resultsAvailable();
+        return true;
+    }
+
+    // request lyrics
+    auto *reply = requestSongDetails(desc);
+    addReply(reply, bind(&LyricsWikiaResultsModel::handleSongDetailsFinished, this, reply, index.row()));
+    return false;
 }
 
 void LyricsWikiaResultsModel::parseInitialResults(const QByteArray &data)

dbquery/lyricswikia.h

@@ -12,12 +12,12 @@ class LyricsWikiaResultsModel : public HttpResultsModel {
 
 public:
     LyricsWikiaResultsModel(SongDescription &&initialSongDescription, QNetworkReply *reply);
-    bool fetchCover(const QModelIndex &index);
+    bool fetchCover(const QModelIndex &index) override;
-    bool fetchLyrics(const QModelIndex &index);
+    bool fetchLyrics(const QModelIndex &index) override;
-    QUrl webUrl(const QModelIndex &index);
+    QUrl webUrl(const QModelIndex &index) override;
 
 protected:
-    void parseInitialResults(const QByteArray &data);
+    void parseInitialResults(const QByteArray &data) override;
 
 private:
     QNetworkReply *requestSongDetails(const SongDescription &songDescription);

dbquery/musicbrainz.cpp

@@ -25,31 +25,38 @@ MusicBrainzResultsModel::MusicBrainzResultsModel(SongDescription &&initialSongDe
 
 bool MusicBrainzResultsModel::fetchCover(const QModelIndex &index)
 {
+    // FIXME: avoid code duplication with lyricswikia.cpp
+
+    // find song description
     if (index.parent().isValid() || index.row() >= m_results.size()) {
         return true;
     }
     SongDescription &desc = m_results[index.row()];
+
+    // skip if cover is already available
     if (!desc.cover.isEmpty()) {
-        // cover is already available -> nothing to do
         return true;
     }
 
+    // fail if album ID is unknown
     if (desc.albumId.isEmpty()) {
         m_errorList << tr("Unable to fetch cover: Album ID unknown");
         emit resultsAvailable();
         return true;
     }
-    try {
+
-        // the item belongs to an album which cover has already been fetched
+    // skip if the item belongs to an album which cover has already been fetched
-        desc.cover = m_coverData.at(desc.albumId);
+    const auto coverData = m_coverData.find(desc.albumId);
-    } catch (const out_of_range &) {
+    if (coverData != m_coverData.end()) {
-        // request the cover art
+        desc.cover = coverData->second;
-        auto *const reply = queryCoverArtArchive(desc.albumId);
+        return true;
-        addReply(reply, bind(&MusicBrainzResultsModel::handleCoverReplyFinished, this, reply, desc.albumId, index.row()));
-        setFetchingCover(true);
-        return false;
     }
-    return true;
+
+    // request the cover art
+    auto *const reply = queryCoverArtArchive(desc.albumId);
+    addReply(reply, bind(&MusicBrainzResultsModel::handleCoverReplyFinished, this, reply, desc.albumId, index.row()));
+    setFetchingCover(true);
+    return false;
 }
 
 QUrl MusicBrainzResultsModel::webUrl(const QModelIndex &index)

dbquery/musicbrainz.h

@@ -16,11 +16,11 @@ private:
 
 public:
     MusicBrainzResultsModel(SongDescription &&initialSongDescription, QNetworkReply *reply);
-    bool fetchCover(const QModelIndex &index);
+    bool fetchCover(const QModelIndex &index) override;
-    QUrl webUrl(const QModelIndex &index);
+    QUrl webUrl(const QModelIndex &index) override;
 
 protected:
-    void parseInitialResults(const QByteArray &data);
+    void parseInitialResults(const QByteArray &data) override;
 
 private:
     What m_what;