From ffe02e59c7a9ef2cc306ba66346e54017b3a089f Mon Sep 17 00:00:00 2001 From: RickyRister <42636155+RickyRister@users.noreply.github.com> Date: Mon, 21 Apr 2025 13:29:42 -0700 Subject: [PATCH] Refactor: clean up OracleImporter (#5871) * remove unused dataDir variable * inline setsMap * join declaration and assignment * make the protected methods static * make getSetPriority static * inline mainCardTypes list and make the method static * pass by const ref when able * rename param to match --- oracle/src/oracleimporter.cpp | 83 +++++++++++++++++------------------ oracle/src/oracleimporter.h | 29 ++++-------- oracle/src/oraclewizard.cpp | 2 +- 3 files changed, 51 insertions(+), 63 deletions(-) diff --git a/oracle/src/oracleimporter.cpp b/oracle/src/oracleimporter.cpp index 22edebac2..7138f80af 100644 --- a/oracle/src/oracleimporter.cpp +++ b/oracle/src/oracleimporter.cpp @@ -12,18 +12,18 @@ SplitCardPart::SplitCardPart(const QString &_name, const QString &_text, const QVariantHash &_properties, - const CardInfoPerSet _setInfo) + const CardInfoPerSet &_setInfo) : name(_name), text(_text), properties(_properties), setInfo(_setInfo) { } const QRegularExpression OracleImporter::formatRegex = QRegularExpression("^format-"); -OracleImporter::OracleImporter(const QString &_dataDir, QObject *parent) : QObject(parent), dataDir(_dataDir) +OracleImporter::OracleImporter(QObject *parent) : QObject(parent) { } -CardSet::Priority OracleImporter::getSetPriority(QString &setType, QString &shortName) +static CardSet::Priority getSetPriority(const QString &setType, const QString &shortName) { if (!setTypePriorities.contains(setType.toLower())) { qDebug() << "warning: Set type" << setType << "unrecognized for prioritization"; @@ -40,30 +40,22 @@ bool OracleImporter::readSetsFromByteArray(const QByteArray &data) QList newSetList; bool ok; - setsMap = QtJson::Json::parse(QString(data), ok).toMap().value("data").toMap(); + auto setsMap = QtJson::Json::parse(QString(data), ok).toMap().value("data").toMap(); if (!ok) { qDebug() << "error: QtJson::Json::parse()"; return false; } QListIterator it(setsMap.values()); - QVariantMap map; - - QString shortName; - QString longName; - QList setCards; - QString setType; - QDate releaseDate; - CardSet::Priority priority; while (it.hasNext()) { - map = it.next().toMap(); - shortName = map.value("code").toString().toUpper(); - longName = map.value("name").toString(); - setCards = map.value("cards").toList(); - setType = map.value("type").toString(); - releaseDate = map.value("releaseDate").toDate(); - priority = getSetPriority(setType, shortName); + QVariantMap map = it.next().toMap(); + QString shortName = map.value("code").toString().toUpper(); + QString longName = map.value("name").toString(); + QList setCards = map.value("cards").toList(); + QString setType = map.value("type").toString(); + QDate releaseDate = map.value("releaseDate").toDate(); + CardSet::Priority priority = getSetPriority(setType, shortName); // capitalize set type if (setType.length() > 0) { // basic grammar for words that aren't capitalized, like in "From the Vault" @@ -93,13 +85,16 @@ bool OracleImporter::readSetsFromByteArray(const QByteArray &data) return true; } -QString OracleImporter::getMainCardType(const QStringList &typeList) +static QString getMainCardType(const QStringList &typeList) { if (typeList.isEmpty()) { return {}; } - for (const auto &type : mainCardTypes) { + static const QStringList typePriority = {"Planeswalker", "Creature", "Land", "Sorcery", + "Instant", "Artifact", "Enchantment"}; + + for (const auto &type : typePriority) { if (typeList.contains(type)) { return type; } @@ -108,12 +103,33 @@ QString OracleImporter::getMainCardType(const QStringList &typeList) return typeList.first(); } +/** + * Sorts and deduplicates the color chars in the string by WUBRG order. + * + * @param colors The string containing the color chars. Will be modified in-place + */ +static void sortAndReduceColors(QString &colors) +{ + // sort + static const QHash colorOrder{{'W', 0}, {'U', 1}, {'B', 2}, {'R', 3}, {'G', 4}}; + std::sort(colors.begin(), colors.end(), + [](const QChar a, const QChar b) { return colorOrder.value(a, INT_MAX) < colorOrder.value(b, INT_MAX); }); + // reduce + QChar lastChar = '\0'; + for (int i = 0; i < colors.size(); ++i) { + if (colors.at(i) == lastChar) + colors.remove(i, 1); + else + lastChar = colors.at(i); + } +} + CardInfoPtr OracleImporter::addCard(QString name, - QString text, + const QString &text, bool isToken, QVariantHash properties, - QList &relatedCards, - CardInfoPerSet setInfo) + const QList &relatedCards, + const CardInfoPerSet &setInfo) { // Workaround for card name weirdness name = name.replace("Æ", "AE"); @@ -197,7 +213,7 @@ CardInfoPtr OracleImporter::addCard(QString name, return newCard; } -QString OracleImporter::getStringPropertyFromMap(const QVariantMap &card, const QString &propertyName) +static QString getStringPropertyFromMap(const QVariantMap &card, const QString &propertyName) { return card.contains(propertyName) ? card.value(propertyName).toString() : QString(""); } @@ -442,23 +458,6 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QList return numCards; } -void OracleImporter::sortAndReduceColors(QString &colors) -{ - // sort - const QHash colorOrder{{'W', 0}, {'U', 1}, {'B', 2}, {'R', 3}, {'G', 4}}; - std::sort(colors.begin(), colors.end(), [&colorOrder](const QChar a, const QChar b) { - return colorOrder.value(a, INT_MAX) < colorOrder.value(b, INT_MAX); - }); - // reduce - QChar lastChar = '\0'; - for (int i = 0; i < colors.size(); ++i) { - if (colors.at(i) == lastChar) - colors.remove(i, 1); - else - lastChar = colors.at(i); - } -} - int OracleImporter::startImport() { int setCards = 0, setIndex = 0; diff --git a/oracle/src/oracleimporter.h b/oracle/src/oracleimporter.h index b50c56d50..314723706 100644 --- a/oracle/src/oracleimporter.h +++ b/oracle/src/oracleimporter.h @@ -92,7 +92,10 @@ public: class SplitCardPart { public: - SplitCardPart(const QString &_name, const QString &_text, const QVariantHash &_properties, CardInfoPerSet setInfo); + SplitCardPart(const QString &_name, + const QString &_text, + const QVariantHash &_properties, + const CardInfoPerSet &setInfo); inline const QString &getName() const { return name; @@ -121,8 +124,6 @@ class OracleImporter : public QObject { Q_OBJECT private: - const QStringList mainCardTypes = {"Planeswalker", "Creature", "Land", "Sorcery", - "Instant", "Artifact", "Enchantment"}; static const QRegularExpression formatRegex; /** @@ -136,27 +137,23 @@ private: SetNameMap sets; QList allSets; - QVariantMap setsMap; - QString dataDir; - QString getMainCardType(const QStringList &typeList); CardInfoPtr addCard(QString name, - QString text, + const QString &text, bool isToken, QVariantHash properties, - QList &relatedCards, - CardInfoPerSet setInfo); + const QList &relatedCards, + const CardInfoPerSet &setInfo); signals: void setIndexChanged(int cardsImported, int setIndex, const QString &setName); void dataReadProgress(int bytesRead, int totalBytes); public: - explicit OracleImporter(const QString &_dataDir, QObject *parent = nullptr); - CardSet::Priority getSetPriority(QString &setType, QString &shortName); + explicit OracleImporter(QObject *parent = nullptr); bool readSetsFromByteArray(const QByteArray &data); int startImport(); bool saveToFile(const QString &fileName, const QString &sourceUrl, const QString &sourceVersion); - int importCardsFromSet(const CardSetPtr ¤tSet, const QList &cards); + int importCardsFromSet(const CardSetPtr ¤tSet, const QList &cardsList); const CardNameMap &getCardList() const { return cards; @@ -165,15 +162,7 @@ public: { return allSets; } - const QString &getDataDir() const - { - return dataDir; - } void clear(); - -protected: - inline QString getStringPropertyFromMap(const QVariantMap &card, const QString &propertyName); - void sortAndReduceColors(QString &colors); }; #endif diff --git a/oracle/src/oraclewizard.cpp b/oracle/src/oraclewizard.cpp index 785835cf4..bea87dde2 100644 --- a/oracle/src/oraclewizard.cpp +++ b/oracle/src/oraclewizard.cpp @@ -60,7 +60,7 @@ OracleWizard::OracleWizard(QWidget *parent) : QWizard(parent) settings = new QSettings(SettingsCache::instance().getSettingsPath() + "global.ini", QSettings::IniFormat, this); connect(&SettingsCache::instance(), &SettingsCache::langChanged, this, &OracleWizard::updateLanguage); - importer = new OracleImporter(SettingsCache::instance().getDataPath(), this); + importer = new OracleImporter(this); nam = new QNetworkAccessManager(this);