Klasa realizująca niestandardową kolejność sortowania

1

Witam Bracia i Siostry w kodzie

Co uważacie na temat klasy jaką wykoncypowałem, aby rozwiązać zagadnienie własnej, niealfabetycznej kolejności sortowania listy obiektów?

customsortorder.h

/* The class determines custom sort order for items displayed in AddEquipmentItem groups. It does so by checking passed InventoryItem field with the first occurence of matching regexp from
 * appropriate vector xxxSortOrderVector.
*/
class CustomSortOrder
{
public:
    static void sortItems(QList<const InventoryItem*> &itemsToSort, const QString &groupName);

private:
    QVector<QString> ballisticSortOrderVector;
    QVector<QString> energySortOrderVector;
    QVector<QString> missileSortOrderVector;
    QVector<QString> supportSortOrderVector;
    QVector<QString> ammoSortOrderVector;
    QVector<QString> gyroSortOrderVector;
    QVector<QString> actuatorsSortOrderVector;
    QVector<QString> computersSortOrderVector;
    QVector<QString> heatsinksSortOrderVector;

    CustomSortOrder();
};

customsortorder.cpp

CustomSortOrder::CustomSortOrder()
{
    ballisticSortOrderVector.append("^AC/2(\\D|$)");
    ballisticSortOrderVector.append("^AC/5(\\D|$)");
    ballisticSortOrderVector.append("^AC/10(\\D|$)");
    ballisticSortOrderVector.append("^AC/20(\\D|$)");
    ballisticSortOrderVector.append("^Gauss Rifle(.|$)");

    //as it turned out additional ^ in regexps was required, otherwise it matched M Laser in ER M Laser
    energySortOrderVector.append("^M Laser(.|$)");
    energySortOrderVector.append("^ER M Laser(.|$)");
    energySortOrderVector.append("^M Pulse(.|$)");
    energySortOrderVector.append("^L Laser(.|$)");
    energySortOrderVector.append("^ER L Laser(.|$)");
    energySortOrderVector.append("^L Pulse(.|$)");
    energySortOrderVector.append("^PPC(.|$)");
    energySortOrderVector.append("^ER PPC(.|$)");

    missileSortOrderVector.append("^SRM2(\\D|$)");
    missileSortOrderVector.append("^SRM4(\\D|$)");
    missileSortOrderVector.append("^SRM6(\\D|$)");
    missileSortOrderVector.append("^LRM5(\\D|$)");
    missileSortOrderVector.append("^LRM10(\\D|$)");
    missileSortOrderVector.append("^LRM15(\\D|$)");
    missileSortOrderVector.append("^LRM20(\\D|$)");

    supportSortOrderVector.append("^S Laser(.|$)");
    supportSortOrderVector.append("^ER S Laser(.|$)");
    supportSortOrderVector.append("^S Pulse(.|$)");
    supportSortOrderVector.append("^MG(.|$)");
    supportSortOrderVector.append("^Flamer(.|$)");

    ammoSortOrderVector.append("^SRM.");
    ammoSortOrderVector.append("^LRM.");
    ammoSortOrderVector.append("^MG.");
    ammoSortOrderVector.append("^AC/2 A.");
    ammoSortOrderVector.append("^AC/5 A.");
    ammoSortOrderVector.append("^AC/10 A.");
    ammoSortOrderVector.append("^AC/20 A.");
    ammoSortOrderVector.append("^Gauss.");

    gyroSortOrderVector.append("^- ");
    gyroSortOrderVector.append(" Hit Defense");
    gyroSortOrderVector.append(" Melee");

    actuatorsSortOrderVector.append("Melee Dmg");
    actuatorsSortOrderVector.append("Stb");
    actuatorsSortOrderVector.append("DFA Dmg");
    actuatorsSortOrderVector.append("DFA Self");

    computersSortOrderVector.append("Ballistic");
    computersSortOrderVector.append("Energy");
    computersSortOrderVector.append("Missile");

    heatsinksSortOrderVector.append("Sink$");
    heatsinksSortOrderVector.append("\\(D\\)$");
    heatsinksSortOrderVector.append("^Exchanger");
    heatsinksSortOrderVector.append("Bank");
}

void CustomSortOrder::sortItems(QList<const InventoryItem*> &itemsToSort, const QString &groupName)
{
    static CustomSortOrder cso;

    std::function<const QString&(const InventoryItem *item)> fieldToSortOn;
    fieldToSortOn = [](const InventoryItem *item)->const QString&
    {
        return item->name;
    };

    QVector<QString> *sortOrderToUse = nullptr;
    if (groupName == Categories::ballistic)
    {
        sortOrderToUse = &cso.ballisticSortOrderVector;
    }
    else if (groupName == Categories::energy)
    {
        sortOrderToUse = &cso.energySortOrderVector;
    }
    else if (groupName == Categories::missile)
    {
        sortOrderToUse = &cso.missileSortOrderVector;
    }
    else if (groupName == Categories::support)
    {
        sortOrderToUse = &cso.supportSortOrderVector;
    }
    else if (groupName == Categories::ammo)
    {
        sortOrderToUse = &cso.ammoSortOrderVector;        
    }
    else if (groupName == Categories::heatsinks)
    {
        sortOrderToUse = &cso.heatsinksSortOrderVector;
    }
    else//non-name based sorting
    {
        if (groupName == Categories::gyros)
        {
            sortOrderToUse = &cso.gyroSortOrderVector;
        }
        else if (groupName == Categories::actuators)
        {
            sortOrderToUse = &cso.actuatorsSortOrderVector;
        }
        else if (groupName == Categories::computers)
        {
            sortOrderToUse = &cso.computersSortOrderVector;
        }
        else//no match to any mentioned groupName, so keep name field as sort base
        {
            goto skipFieldChange;//jump down
        }

        fieldToSortOn = [](const InventoryItem *item)->const QString&
        {
            return item->bonusValueA;
        };
    }

skipFieldChange:
    std::function<bool(const InventoryItem *item1, const InventoryItem *item2)> sortLambda;
    sortLambda = [sortOrderToUse, fieldToSortOn](const InventoryItem *item1, const InventoryItem *item2)->bool
    {
        QString field1 = fieldToSortOn(item1);
        QString field2 = fieldToSortOn(item2);

        if (sortOrderToUse != nullptr && sortOrderToUse->size() > 0)
        {
            QRegularExpression regExp;
            regExp.setPatternOptions(QRegularExpression::CaseInsensitiveOption);

            int index1 = -1, index2 = -1;
            for (int cnt = 0; cnt < sortOrderToUse->size(); cnt++)
            {
                regExp.setPattern(sortOrderToUse->value(cnt));

                if (index1 < 0 && regExp.match(field1).hasMatch())
                {
                    index1 = cnt;
                }

                if (index2 < 0 && regExp.match(field2).hasMatch())
                {
                    index2 = cnt;
                }

                if (index1 >= 0 && index2 >= 0)
                {
                    break;
                }
            }

            //both fields are found in sort order vector
            if (index1 >= 0 && index2 >= 0)
            {
                if (index1 != index2)
                {
                    return index1 < index2;
                }

                //in the case of same InventoryItem fields the one with shorter name should be considered lesser
                if (field1.size() != field2.size())
                {
                    return fieldToSortOn(item1).size() < fieldToSortOn(item2).size();
                }
            }

            //if only one of the passed InventoryItems matched regexp stored in sort order vector the one which has match should be considered smaller
            if (index1 >=0 && index2 < 0)
            {
                return true;
            }
            if (index1 < 0 && index2 >= 0)
            {
                return false;
            }
        }

        //preform ordinary alphabetical sort for equipment names not included in sortOrderVectors
        //return field1 < field2;
        return item1->name < item2->name;
    };

    std::sort(itemsToSort.begin(), itemsToSort.end(), sortLambda);
}
3
    QVector<QString> ballisticSortOrderVector;
    QVector<QString> energySortOrderVector;
    QVector<QString> missileSortOrderVector;
    QVector<QString> supportSortOrderVector;
    QVector<QString> ammoSortOrderVector;
    QVector<QString> gyroSortOrderVector;
    QVector<QString> actuatorsSortOrderVector;
    QVector<QString> computersSortOrderVector;
    QVector<QString> heatsinksSortOrderVector;

Czy gdybyś miał 25 kategorii, stworzyłbyś 25 pól?
Jak podszedłbyś do tematu w sytuacji, gdyby to użytkownik miał mieć możliwość tworzenia własnych kategorii sortowania?

Personalnie zmieniłbym też nazwę klasy CustomSortOrder na coś bliżej ProductsSorter.

0
Patryk27 napisał(a):

Czy gdybyś miał 25 kategorii, stworzyłbyś 25 pól?

Pewnie bym stworzył jeśli z góry wiadomo by było, że będzie ich właśnie tyle. Jeśli jednak owych kategorii miałaby być zmienna liczba, to raczej zastosowałbym mapę <"nazwa kategorii", wektor z regexpami>

Patryk27 napisał(a):

Jak podszedłbyś do tematu w sytuacji, gdyby to użytkownik miał mieć możliwość tworzenia własnych kategorii sortowania?

Wdrożyłbym pierwotny pomysł którym było wczytywanie regexpów z pliku, a którego zaniechałem ponieważ do moich potrzeb wpisywanie regexpów z palca w konstruktorze jest wystarczające. Btw, wówczas i powyższe rozwiązanie dotyczące wprowadzenia mapy wektorów tym bardziej nabrałoby sensu.

0

Każda lambda która jest dłuższa niż 5 linijek IMO nie powinna być lambdą tylko funkcją/metodą.
Kod nie jest najwyższej jakości więc trudno się czyta co ma robić. Mógłbyś wytłumaczyć czym jest "niealfabetyczna kolejność", jaką relację mają spełniać posortowane obiekty?
Wtedy łatwiej będzie zrozumieć kod.

0

Kolejność posortowania określa indeks w stosownym do kategorii wektorze, a określenie to robi się poprzez test regexpa z tegoż indeksu vs przesłane InventoryItemy, przy uwzględnieniu po którym polu InventoryItemy mają być sprawdzane (lambda fieldToSortOn).
Przykładowe dane dla kategorii Ballistic:
AC/2
AC/2 +
AC/2 + +
AC/2 + + +
AC/5
AC/5 +
AC/5 + +
AC/5 + + +
AC/10
AC/10 +
AC/10 + +
AC/10 + + +
AC/20
AC/20 +
AC/20 + +
AC/20 + + +
Gauss Rifle
Gruba Rura -> której nie ma wektorze, sortowanie alfabetyczne za danymi które się na regexpy załapały
Wielkie Działo -> też łapie się na ostatni warunek sortujący alfabetycznie

1

Dziwny koncept. Mi to wygląda na skopany koncept, bo InventoryItem powinien mieć pola, które pozwalają na sortownie bez RE.
Takie uwiązanie do opisu przedmiotu jest bardzo delikatne, po pierwsze literówki, a po drugie potencjalna lokalizacja (nie wiadomo czy tłumacz utrzyma konsystencję napisów, albo czy nawet będzie można to obsłużyć wyrażeniami regularnymi w innym języku).
Z tego powodu uważam sam pomysł za bardzo nietrafiony i prowadzący do trudnych do opanowania błędów (nie mówiąc o tym, że jest to wydajnościowa porażka).

Ale ja już to robić w ten sposób to trzeba zorganizować kod w sprytniejszy sposób.
Ja bym stworzył taką klasę (coś w ten deseń):

class ICompareString {
public:
    ~ICompareString() {}

     int compare(const QString &, const QString &) const;
};

class RegExpPririorityComparator : public ICompareString 
{
public:
    RegExpPririorityComparator(ICompareString *fallback, std::initializer_list<QRegularExpression> reIniList)
        : mRegExp{ reIniList }
    {
    }

    override int compare(const QString &a, const QString &b) const {
         for (const auto &re : mRegExp) {
               auto m1 = re.match(a).hasMatch();
               auto m2 = re.match(b).hasMatch();
               if (m1 && m2) {
                    return mFallback->compare(a, b);
               }
               if (m1) return 1;
               if (m2) return -1;
         }
         return mFallback->compare(a, b);
    }

public:
    QVector<QRegularExpression> mRegExp;
    ICompareString  *mFallback; // a może shared_ptr?
};

Potem użyłbym tego jako klocka do budowania czegoś bardziej zaawansowanego.

0
MarekR22 napisał(a):

Dziwny koncept. Mi to wygląda na skopany koncept, bo InventoryItem powinien mieć pola, które pozwalają na sortownie bez RE.

No i posiada, bo widoczne w kodzie pola name i bonusValueA są typu QString -> więc nic nie stoi na przeszkodzie, aby sortować je bez udziwnień w sytuacji, jak wartość pola nie pasuje do żadnego z regexpów.

MarekR22 napisał(a):

Takie uwiązanie do opisu przedmiotu jest bardzo delikatne, po pierwsze literówki, a po drugie potencjalna lokalizacja (nie wiadomo czy tłumacz utrzyma konsystencję napisów, albo czy nawet będzie można to obsłużyć wyrażeniami regularnymi w innym języku).

Rozwiązanie jest przystosowane do konkretnego przypadku, nie wszystkich możliwych i potencjalnych. Dane kategorii Ballistic podałem, jeśli widzisz Marku potrzebę podam resztę.
Jeśli jednak patrzeć z perspektywy generyczności wówczas fakt, takie uwagi nabierają znaczenia.

MarekR22 napisał(a):

Z tego powodu uważam sam pomysł za bardzo nietrafiony i prowadzący do trudnych do opanowania błędów (nie mówiąc o tym, że jest to wydajnościowa porażka).

Bardzo mnie ciekawi czemu jest to wydajnościowa porażka?
Co do błędów, ich konsekwencje będą praktycznie żadne, po prostu przedmiot posortuje się alfabetycznie po nazwie.

Ale ja już to robić w ten sposób to trzeba zorganizować kod w sprytniejszy sposób.
Ja bym stworzył taką klasę (coś w ten deseń): (i tu blok kodu)
Potem użyłbym tego jako klocka do budowania czegoś bardziej zaawansowanego.

Hmmmm...Bracie Marku, to co podałeś raczej nie zadziała - w porównaniu if (m1 && m2) użyty jest mFallback->compare(), aby na pewno o fallback chodziło? Oj, chyba nie...skoro już się wgryzasz w temat to rozwiń ten przykład, aby się dało zobaczyć w kodzie efekt działania - na razie to taki szkic pomysłu dopiero.
Ale, będziesz potrzebował danych wejściowych. To tak:

struct InventoryItem
{
  QString name;
  QString bonusValueA;
};
//w rzeczywistości ma ona pól ze 4x tyle, ale do sortowania korzystam z tych dwóch; z bonusValueA w sytuacji, kiedy nazwa jest taka sama

ze 2 inne kategorie oprócz tej Ballistic - oto jakie nazwy w nich występują, i jak mają być sortowane:
kategoria Missiles, nazwy i wynikowe sortowanie:
SRM2
SRM2 +
SRM2 + +
SRM2 + + +
SRM4
SRM4 +
SRM4 + +
SRM4 + + +
SRM6
SRM6 +
SRM6 + +
SRM6 + + +
LRM5
LRM5 +
LRM5 + +
LRM5 + + +
LRM10
LRM10 +
LRM10 + +
LRM10 + + +
LRM15
LRM15 +
LRM15 + +
LRM15 + + +
LRM20
LRM20 +
LRM20 + +
LRM20 + + +

kategoria Computers
nazwy szystkie mają TTS, TTS + oraz TTS + +, a bonusValueA to od +1 do +3 Accuracy (typ uzbrojenia). Wynikowe sortowanie:
TTS
+1 Accuracy (Ballistic)
TTS +
+2 Accuracy (Ballistic)
TTS + +
+3 Accuracy (Ballistic)
TTS
+1 Accuracy (Energy)
TTS +
+2 Accuracy (Energy)
TTS + +
+3 Accuracy (Energy)
TTS
+1 Accuracy (Missile)
TTS +
+2 Accuracy (Missile)
TTS + +
+3 Accuracy (Missile)

Bo widzisz Marku, w moim rozwiązaniu aby posortować listy InventoryItemów według własnych wytycznych wystarczy w kodzie użyć:

CustomSortOrder::sortItems(listaDoPosortowania, "nazwa kategorii");

czy Twoje rozwiązanie będzie w stanie równać się z taką prostotą użycia?

0

Czemu (.|$) nie może być po prostu .?

0
TomRiddle napisał(a):

Czemu (.|$) nie może być po prostu .?

No właśnie nie, inaczej nie łapało poprawnie przypadku: AC/2 vs AC/20.
A nie, wróć, to nie przy tym jest używane. Który przypadek masz Bracie @TomRiddle na myśli?

1
MasterBLB napisał(a):

Który przypadek masz Bracie @TomRiddle na myśli?

    energySortOrderVector.append("^M Laser(.|$)");
    energySortOrderVector.append("^ER M Laser(.|$)");
    energySortOrderVector.append("^M Pulse(.|$)");
    energySortOrderVector.append("^L Laser(.|$)");
    energySortOrderVector.append("^ER L Laser(.|$)");
    energySortOrderVector.append("^L Pulse(.|$)");
    energySortOrderVector.append("^PPC(.|$)");
    energySortOrderVector.append("^ER PPC(.|$)");

Zastanawiam się czemu ^M Laser(.|$) nie mogłoby się stać ^M Laser.? W ogóle caly ten regexp jest dziwny. Albo dowolny znak albo koniec? Osobliwe.

1

Zamiast (.|$) powinno być po prostu: .?.
Zresztą ^ na początku też jest zbędne skoro używane jest match a nie find.

0

@TomRiddle: - Hmm, pewnie z rozpędu po stworzeniu regexpów dla kategorii ballistic, no i dawno się regexpami nie bawiłem. Zaraz obczaję, czy faktycznie nie dałoby się uprościć.
Natomiast ^ okazuje się być konieczny @MarekR22, patrz komentarz nad wektorem dla energy. No i nadal nie napisałeś, gdzie w tym podejściu jest użynanie wydajności.

EDIT:
Faktycznie, w wersji uproszczonej też działa. Dzięki chłopaki!

1

Dlaczego nie zdefiniujesz przedmiotów jako pary: nazwaPrzedmiotu, kolejnoscSortowania?

np.
Przedmiot1, 10
Przedmiot2, 20
Przedmiot3, 30

A potem sortujesz po polu kolejnoscSortowania zamiast kombinowania z regexpami (a jak się postać nazwy zmieni, to będziesz przerabiać program?).

0

Głównie dlatego, że jako dane wejściowe z wielu miejsc aplikacji dostaję QList<const InventoryItem*>& - teraz przerabianie tego na listę par to sporo zachodu, a i tak potem pewnie trzeba by się par pozbywać bo te inne miejsca aplikacji oczekują listy wskaźników na InventoryItemy. Tymczasem dzięki sztuczce z leniwą inicjalizacją statycznego obiektu cso wektory w konstruktorze utworzą się tylko raz, a potem pozostaną w pamięci gotowe do kolejnego użytku, a i zawartość listy pozostaje bez zmian - poza zmianą kolejności rzecz jasna.
Poza tym mam ten komfort iż wiem, że te nazwy co występują się nie zmienią (no, najwyżej pojawi się np M Laser z 4 lub więcej plusami, teraz maks to 3), co najwyżej dojdą nowe.

Za to podejście takie jak proponujesz miałoby spore zalety aby własną kolejność sortowania określać dynamicznie, w trakcie działania programu.

W sumie to nie wiem, czy nie powinienem opisać dokładnie warunków na wejściu oraz zakresu danych na jakich ta klasa działa, bo widzę Bracia iż rozpatrujecie to w szerszym kontekście i słusznie zauważacie różne przeszkody, jakie się pojawiają jeśli by chcieć zastosować CustomSortOrder gdzieś indziej. A ja ją zwyczajnie zaprojektowałem do poradzenia sobie z konkretnym zadaniem, bez konieczności uwzględnienia np. dziedziczenia jej.

0

Ja się przyczepię do kilku rzeczy:

  1. Ściana appendów -> nie lepiej użyć std::initializer_list (https://doc.qt.io/qt-5/qvector.html#QVector-5) - raz, że mniej kodu, dwa, że czytelniej i szybciej się wykona
  2. Ta drabina ifów z goto na końcu -> ciężko mi zrozumieć Twoje intencje tutaj, ale jakbyś tak wrzucił te wektory do mapy z kluczem jako kategoria to całą tę drabinę być zamienił na jedną linię.
  3. goto? Ja wiem, że miewa to czasami uzasadnienie (bardzo rzadko), ale tutaj wystarczy sprawdzić czy sortOrderToUse ma wartość null, żeby ominąć ustawianie tej lambdy
  4. Abstrachując już od sensowności drugiej lambdy, to zdajesz sobie sprawę, że przy tej wielkości funkcji std::function niemal na pewno zaalokuje to na stercie? Za każdym razem jak wywołasz funkcję sortItems?
  5. Na koniec nie rozumiem, jaki jest sens w tworzeniu statycznego obiektu w statycznej klasie po to tylko aby użyć niestatycznych wektorów z tej klasy. Nie prościej po prostu zrobić te wektory statycznymi?

Z takich grubszych technicznych rzeczy to tyle mi się na razie rzuciło.

0

Dzięki za uwagi Bracie @trickle
ad 1) Być może, tyle, że ja std::initializer_list nie znam. Przyjrzę się tematowi cóż to takiego.
ad 2) Wtedy miałbym drabinę insert-ów to mapy, aczkolwiek zysk byłby taki, że mógłbym przesunąć to z lambdy do konstruktora. W chwili jak klasę projektowałem pod bardzo konkretne potrzeby było to ok, ale kroi się, że dojdą mi kolejne kategorie, oraz że prawdopodobnie trzeba będzie dać użytkownikom możliwość zmiany kolejności.
ad 3) Tu pełna racja, czemu sam nie zauważyłem? No nic, dzięki za wyłapanie, poprawione.
ad 4) Niestety, nie. Zabrzmi to dziwacznie, ale C++11 dopiero poznaję bo projekty w których pracowałem nie wykorzystywały tego standardu. Jeśli znasz jakieś linki gdzie temat lambd C++ jest naprawdę dogłębnie przedstawiony to bardzo chętnie się z nimi zapoznam.
Co do alokowania na stercie za każdym razem, to jeszcze by w miarę uszło pod warunkiem, że za każdym też razem nastąpi dealokacja. Niemniej fakt, w takim przypadku trzepać za każdym razem new/delete to tak trochę nie bardzo, lepiej będzie zrobić z tego zwykłą funkcję.
ad 5) Dla prostoty zapisu CustomSortOrder::sortItems() który ponadto zapewni leniwą inicjalizację wektorów sortowań przy pierwszym użyciu. W takim wypadku istotnie, wektory równie dobrze mogłyby być statyczne.

1
MasterBLB napisał(a):

ad 2) Wtedy miałbym drabinę insert-ów to mapy, aczkolwiek zysk byłby taki, że mógłbym przesunąć to z lambdy do konstruktora. W chwili jak klasę projektowałem pod bardzo konkretne potrzeby było to ok, ale kroi się, że dojdą mi kolejne kategorie, oraz że prawdopodobnie trzeba będzie dać użytkownikom możliwość zmiany kolejności.
ad 4) Niestety, nie. Zabrzmi to dziwacznie, ale C++11 dopiero poznaję bo projekty w których pracowałem nie wykorzystywały tego standardu. Jeśli znasz jakieś linki gdzie temat lambd C++ jest naprawdę dogłębnie przedstawiony to bardzo chętnie się z nimi zapoznam.

QMap też ma konstruktor przyjmujący std: https://doc.qt.io/Qt-5/qmap.html#QMap-1 - więc insertów też nie będzie
Jeśli chodzi o std::function to tu masz przykładowy artykuł: https://blog.demofox.org/2015/02/25/avoiding-the-performance-hazzards-of-stdfunction/
Natomiast na temat lambd (ich implementacji): https://web.mst.edu/~nmjxv3/articles/lambdas.html

1 użytkowników online, w tym zalogowanych: 0, gości: 1