[Refactor] Clean up CardDatabase (#6034)

* Inline getCardFromMap

map.value already returns a default-constructed value if the key is not present

* Use for-each instead of iterator

* Add new method

* clean up method order

* fix build failure

* clean up getPreferredPrinting usage

* early returns

cockatrice/src/deck/deck_loader.cpp

@@ -324,9 +324,7 @@ struct SetProviderIdToPreferred
     void operator()(const InnerDecklistNode *node, DecklistCardNode *card) const
     {
         Q_UNUSED(node);
-        PrintingInfo preferredPrinting = CardDatabaseManager::getInstance()->getSpecificPrinting(
+        PrintingInfo preferredPrinting = CardDatabaseManager::getInstance()->getPreferredPrinting(card->getName());
-            card->getName(),
-            CardDatabaseManager::getInstance()->getPreferredPrintingProviderIdForCard(card->getName()));
         QString providerId = preferredPrinting.getProperty("uuid");
         QString setShortName = preferredPrinting.getSet()->getShortName();
         QString collectorNumber = preferredPrinting.getProperty("num");

cockatrice/src/game/cards/card_database.cpp

@@ -44,11 +44,9 @@ void CardDatabase::clear()
 {
     clearDatabaseMutex->lock();
 
-    QHashIterator<QString, CardInfoPtr> i(cards);
+    for (const auto &card : cards.values()) {
-    while (i.hasNext()) {
+        if (card) {
-        i.next();
+            removeCard(card);
-        if (i.value()) {
-            removeCard(i.value());
         }
     }
 
@@ -113,14 +111,14 @@ void CardDatabase::removeCard(CardInfoPtr card)
 
 CardInfoPtr CardDatabase::getCard(const QString &cardName) const
 {
-    return getCardFromMap(cards, cardName);
+    return cards.value(cardName);
 }
 
 QList<CardInfoPtr> CardDatabase::getCards(const QStringList &cardNames) const
 {
     QList<CardInfoPtr> cardInfos;
     for (const QString &cardName : cardNames) {
-        CardInfoPtr ptr = getCardFromMap(cards, cardName);
+        CardInfoPtr ptr = cards.value(cardName);
         if (ptr)
             cardInfos.append(ptr);
     }
@@ -162,7 +160,7 @@ CardInfoPtr CardDatabase::getCardByNameAndProviderId(const QString &cardName, co
 
 CardInfoPtr CardDatabase::getCardBySimpleName(const QString &cardName) const
 {
-    return getCardFromMap(simpleNameCards, CardInfo::simplifyName(cardName));
+    return simpleNameCards.value(CardInfo::simplifyName(cardName));
 }
 
 CardInfoPtr CardDatabase::guessCard(const QString &cardName, const QString &providerId) const
@@ -198,22 +196,12 @@ void CardDatabase::addSet(CardSetPtr set)
 SetList CardDatabase::getSetList() const
 {
     SetList result;
-    QHashIterator<QString, CardSetPtr> i(sets);
+    for (auto set : sets.values()) {
-    while (i.hasNext()) {
+        result << set;
-        i.next();
-        result << i.value();
     }
     return result;
 }
 
-CardInfoPtr CardDatabase::getCardFromMap(const CardNameMap &cardMap, const QString &cardName) const
-{
-    if (cardMap.contains(cardName))
-        return cardMap.value(cardName);
-
-    return {};
-}
-
 LoadStatus CardDatabase::loadFromFile(const QString &fileName)
 {
     QFile file(fileName);
@@ -312,6 +300,11 @@ void CardDatabase::refreshPreferredPrintings()
 PrintingInfo CardDatabase::getPreferredPrinting(const QString &cardName) const
 {
     CardInfoPtr cardInfo = getCard(cardName);
+    return getPreferredPrinting(cardInfo);
+}
+
+PrintingInfo CardDatabase::getPreferredPrinting(const CardInfoPtr &cardInfo) const
+{
     if (!cardInfo) {
         return PrintingInfo(nullptr);
     }
@@ -404,15 +397,16 @@ PrintingInfo CardDatabase::getSpecificPrinting(const QString &cardName,
 QString CardDatabase::getPreferredPrintingProviderIdForCard(const QString &cardName)
 {
     PrintingInfo preferredPrinting = getPreferredPrinting(cardName);
-    QString preferredPrintingProviderId = preferredPrinting.getProperty(QString("uuid"));
+    QString uuid = preferredPrinting.getProperty("uuid");
-    if (preferredPrintingProviderId.isEmpty()) {
+    if (!uuid.isEmpty()) {
-        CardInfoPtr defaultCardInfo = getCard(cardName);
+        return uuid;
-        if (defaultCardInfo.isNull()) {
-            return cardName;
-        }
-        return defaultCardInfo->getName();
     }
-    return preferredPrintingProviderId;
+
+    CardInfoPtr defaultCardInfo = getCard(cardName);
+    if (defaultCardInfo.isNull()) {
+        return cardName;
+    }
+    return defaultCardInfo->getName();
 }
 
 bool CardDatabase::isProviderIdForPreferredPrinting(const QString &cardName, const QString &providerId)
@@ -470,9 +464,8 @@ void CardDatabase::refreshCachedReverseRelatedCards()
 QStringList CardDatabase::getAllMainCardTypes() const
 {
     QSet<QString> types;
-    QHashIterator<QString, CardInfoPtr> cardIterator(cards);
+    for (const auto &card : cards.values()) {
-    while (cardIterator.hasNext()) {
+        types.insert(card->getMainCardType());
-        types.insert(cardIterator.next().value()->getMainCardType());
     }
     return types.values();
 }
@@ -480,10 +473,9 @@ QStringList CardDatabase::getAllMainCardTypes() const
 QMap<QString, int> CardDatabase::getAllMainCardTypesWithCount() const
 {
     QMap<QString, int> typeCounts;
-    QHashIterator<QString, CardInfoPtr> cardIterator(cards);
 
-    while (cardIterator.hasNext()) {
+    for (const auto &card : cards.values()) {
-        QString type = cardIterator.next().value()->getMainCardType();
+        QString type = card->getMainCardType();
         typeCounts[type]++;
     }
 
@@ -493,10 +485,9 @@ QMap<QString, int> CardDatabase::getAllMainCardTypesWithCount() const
 QMap<QString, int> CardDatabase::getAllSubCardTypesWithCount() const
 {
     QMap<QString, int> typeCounts;
-    QHashIterator<QString, CardInfoPtr> cardIterator(cards);
 
-    while (cardIterator.hasNext()) {
+    for (const auto &card : cards.values()) {
-        QString type = cardIterator.next().value()->getCardType();
+        QString type = card->getCardType();
 
         QStringList parts = type.split(" — ");
 

cockatrice/src/game/cards/card_database.h

@@ -52,7 +52,6 @@ protected:
     QVector<ICardDatabaseParser *> availableParsers;
 
 private:
-    CardInfoPtr getCardFromMap(const CardNameMap &cardMap, const QString &cardName) const;
     void checkUnknownSets();
     void refreshCachedReverseRelatedCards();
 
@@ -65,15 +64,20 @@ public:
     ~CardDatabase() override;
     void clear();
     void removeCard(CardInfoPtr card);
+
     [[nodiscard]] CardInfoPtr getCard(const QString &cardName) const;
     [[nodiscard]] QList<CardInfoPtr> getCards(const QStringList &cardNames) const;
     QList<CardInfoPtr> getCardsByNameAndProviderId(const QMap<QString, QString> &cardNames) const;
     [[nodiscard]] CardInfoPtr getCardByNameAndProviderId(const QString &cardName, const QString &providerId) const;
+
     [[nodiscard]] PrintingInfo getPreferredPrinting(const QString &cardName) const;
+    [[nodiscard]] PrintingInfo getPreferredPrinting(const CardInfoPtr &cardInfo) const;
     [[nodiscard]] PrintingInfo getSpecificPrinting(const QString &cardName, const QString &providerId) const;
     PrintingInfo
     getSpecificPrinting(const QString &cardName, const QString &setShortName, const QString &collectorNumber) const;
     QString getPreferredPrintingProviderIdForCard(const QString &cardName);
+    bool isProviderIdForPreferredPrinting(const QString &cardName, const QString &providerId);
+
     [[nodiscard]] CardInfoPtr guessCard(const QString &cardName, const QString &providerId = QString()) const;
 
     /*
@@ -83,7 +87,6 @@ public:
     [[nodiscard]] CardInfoPtr getCardBySimpleName(const QString &cardName) const;
 
     CardSetPtr getSet(const QString &setName);
-    bool isProviderIdForPreferredPrinting(const QString &cardName, const QString &providerId);
     static PrintingInfo getSetInfoForCard(const CardInfoPtr &_card);
     const CardNameMap &getCardList() const
     {