[Code Snippet] Save / Load Lists

[NikM]

Falls ihr vor der selben Problemstellung steht wie ich vor einiger Zeit ^^
Hier ist die Lösung :P

//Nur eine Beispiel Struktur die ich für ein Programm genutzt habe
typedef struct
{
    char szItem [256];
    float fGewicht;
} sItem;

template <typename T>
void SaveList (list <T> listX, char *szFileName)
{
    ofstream Output (szFileName, ios::binary); //Output file erstellen
    int iSize = listX.size (); //Größe der übergebenen Liste bestimmen
    list <T>::iterator listIt; //Iterator für die Liste erstellen

    if (Output != NULL) //Nur wenn ein File erstellt wurde
    {
        Output.write ((char *)(&iSize), sizeof (iSize)); //Größe der Liste wird als erstes geschrieben

        for (listIt = listX.begin () ; listIt != listX.end () ; ++listIt) //Die Liste iterieren
            Output.write ((char *)(& (*listIt)), sizeof (T)); //Jedes Item in das File schreiben

        Output.close (); //File schließen
    }
}

template <typename T>
list <T> LoadList (list <T> listX, char *szFileName)
{
    ifstream Input (szFileName, ios::binary);  //Input File suchen
    int iSize = 0;
    list <T> lTempList; //Liste erstellen die nachher zurückgegeben wird
    lTempList.clear (); //Leeren (nicht zwingend nötig)
    T TempItem;

    if (Input != NULL) //Wenn ein Input File gefunden wurde
    {
        Input.read ((char *)(&iSize), sizeof (iSize)); //Als erstes die Größe einlesen

        for (int i = 0 ; i < iSize ; ++i) //Alle Items loopen
        {
            Input.read ((char *)(&TempItem), sizeof (T)); //Item einlesen
            lTempList.push_back (TempItem); //In Liste einfügen
        }

        Input.close (); //File schließen
    }

    return (lTempList); //Erstellte Liste zurückgeben
}

[xNopex]

Ein paar Anmerkungen meinerseits:

void SaveList (list <T> listX, char *szFileName)

C++ -> std::string. Wenn du schon in C++ programmierst, wieso dann C-Strings verwenden?

if (Output != NULL)

Hab ich vorher auch noch nicht gesehen und imho auch nicht sehr schön. Lieber mit fstream::good() arbeiten.

(& (*listIt))

Naja.. Unnötig oder?

[NikM]

1. Ich hab mich an die C-Strings gewöhnt und verweile gerne dabei.
2. Wenn keine Datei gefunden wird bleibt das Objekt == NULL. Die good Methode war mir ehrlichgesagt nicht bekannt ^^
3. Richtig. Ich trenne aber gerne durch Klammern ab um den Code für mich besser lesbar zu machen :P

[.Infinite]

Richtig. Ich trenne aber gerne durch Klammern ab um den Code für mich besser lesbar zu machen :P

Ich glaube das "unnötig" bezog sich eher auf das &* und nicht auf die Klammern...

[xNopex]

Ich hab mich an die C-Strings gewöhnt und verweile gerne dabei.

Völlig falsche Haltung. Wenn du in C++ programmieren willst, hast du dich mit den Elementen der Sprache auseinanderzusetzen. Andernfalls, programmierst du eben in C, wo du C-Strings verwenden kannst.
In C++ C-Strings zu verwenden ist so ziemlich das dümmste, was man machen kann...

2. Wenn keine Datei gefunden wird bleibt das Objekt == NULL.

Ein Objekt kann nicht NULL sein. NULL wird bei Zeigern verwendet, die auf kein Objekt zeigen.

Die good Methode war mir ehrlichgesagt nicht bekannt

Wie gesagt, du musst dich mit den Sprachelementen auseinandersetzen. D.h. Docu lesen, Docu lesen, Docu lesen...

Richtig. Ich trenne aber gerne durch Klammern ab um den Code für mich besser lesbar zu machen :P

Mir gehts nicht um die Klammern..

[jacky919]

Eine Alternative zu good() wäre auch der Negierungsoperator.
Außerdem funktioniert der Vergelich mit NULL sehr wohl, da die Klasse implizit in void* gecastet wird. Der Zeiger ist beim erfolgreichen Laden der Datei nicht 0, sonst schon.

[xNopex]

Dass es auch so funktioniert war mir bewusst. Warum konnte ich mir bislang nicht erklären, dank deiner Erklärung jetzt schon. Trotzdem finde ich es sehr hässlich.

[jacky919]

Originally Posted by xNopex

Dass es auch so funktioniert war mir bewusst. Warum konnte ich mir bislang nicht erklären, dank deiner Erklärung jetzt schon. Trotzdem finde ich es sehr hässlich.

Eigentlich sollte man NULL in gutem C++ ja eh nicht verwenden. Die Nutzung von good() ist sicherlich besser und einleuchtender.

[NikM]

Das &* ist keineswegs unnötig, da ich an die Werte des Iterators möchte und nicht an seine Adresse.

Außerdem funktioniert der Vergelich mit NULL sehr wohl, da die Klasse implizit in void* gecastet wird.

Danke für die Erklärung

[MrSm!th]

Originally Posted by xNopex

Völlig falsche Haltung. Wenn du in C++ programmieren willst, hast du dich mit den Elementen der Sprache auseinanderzusetzen. Andernfalls, programmierst du eben in C, wo du C-Strings verwenden kannst.
In C++ C-Strings zu verwenden ist so ziemlich das dümmste, was man machen kann...

Warum? Die Schnittmenge gibt es nicht umsonst :>


@Code:

Warum iterierst du manuell durch eine std::list? Dafür gibts doch die schönen Iteratoren.

[Nightblizard]

Originally Posted by MrSm!th

Warum? Die Schnittmenge gibt es nicht umsonst :>

Die gibt es, um abwärtskompatibel zu sein. Nach heutigen Standards lässt man die Finger von Arrays, wenn es nicht einen sehr guten Grund gibt sie zu nutzen. Selbst Pointer werden inzwischen durch Klassen, die die Ressourcen "überwachen", verdrängt.
Dafür gibt es verdammt viele gute Gründe! Ich will nicht zu weit ins Detail gehen, aber einen will ich dann doch nennen: Sicherheit!
Falls es jemanden interessiert -> googled es. Dazu gibt es viele Themen im Internet, die dieses Thema behandeln.

@Topic:
Bis auf ein paar Dinge ist das nicht übel!

Du solltest wirklich mehr auf C++ bauen. Vorallem auf C++11! Auch wundert es mich ein wenig, dass du std::list gewählt hast, da der Container nur in wenigen Situationen wirklich gerechtfertigt ist und weil das ganze mit std::vector z.B. noch schöner funktioniert hätte.

Lass mich mal flott den Code durchgehen und dir ein paar Tipps geben.

template <typename T>
void SaveList (list <T> listX, char *szFileName)

Du solltest die list als const reference übergeben. So wie du das jetzt machst wird eine neue Liste erstellt und der Inhalt aus der einen in die neu erstellte kopiert, wenn du die Funktion aufrufen tust und das ist bei einer Liste mit z.B. 10.000 Einträgen sehr zeitintensiv!
Char-Arrays, bzw. C-Strings solltest du in C++ nicht nutzen. Stattdessen einen std::string als const reference übergeben.

    int iSize = listX.size ();
    list <T>::iterator listIt; //Iterator für die Liste erstellen

size ist nicht vom Typ int! Das mag dir Compiler im x86 Modus zwar verzeihen, aber erstellst du das Ganze im x64 Modus regnet es Warnungen, bzw. werden wirklich große Listen erst gar nicht komplett abgespeichert! size_t wäre die richtige Wahl und noch besser wäre auto!
Da du die Iteratoren nicht bearbeitest, solltest du const_iterator wählen.

    if (Output != NULL) //Nur wenn ein File erstellt wurde

Hier solltest du "Output.is_open()" nutzen, da es einfach aussagekräftiger ist.

    {
        Output.write ((char *)(&iSize), sizeof (iSize)); //Größe der Liste wird als erstes geschrieben

C-Casts solltest du in C++ nicht nutzen. Ich weiß, die sind kürzer und angenehmer zu schreiben, aber auch ziemlich unsicher. reinterpret_cast ist hier angesagt!

        for (listIt = listX.begin () ; listIt != listX.end () ; ++listIt) //Die Liste iterieren

std::for_each in Verbindung mit einem Lambda ist hier angesagt. Oder, wenn du VS11 oder den aktuellen gcc nutzt, eine range-based for loop!


Nun zur anderen Funktion! Die Dinge, die bereits in der anderen Funktion erwähnt wurden zähle ich hier nicht nocheinmal auf, sie gelten nichtsdestotrotz!

template <typename T>
list <T> LoadList (list <T> listX, char *szFileName)

listX ist unnötig. Das müsste der Optimierer zwar entfernen, aber schön ist es deswegen nicht. Außerdem würde ich die list nicht per value zurückgeben, da das bei einigen tausend Einträgen einen Flaschenhals ergibt. Hier würde ich den Rückgabetypen als void definieren und aus listX eine Referenz machen. Die übergebene Liste befüllst du nun. So wird nicht die ganze Liste zurückgegeben, sondern lediglich 4 Bytes (oder 8 in x64) an die Funktion übergeben!

Allgemeine Dinge zu deinem Style:
Variablennamen werden klein geschrieben, damit man sie von (groß geschriebenen) Klassen unterscheiden kann.

* und & gehöhren an den Typen, da es eine Eigenschaft des Typens ist und nicht des Namens.

Finger weg vom globalen using namepsace! Dann musst du auch nicht deinen Parametern ein sinnloses X anhängen, damit es nicht zu Problemen kommt.

Der Name iSize ist nicht eindeutig und lässt im schlimmsten Fall sogar vermuten, dass es sich dabei um die Größe in Bytes handelt!

Überarbeitet sollte das Ganze nun in etwa so aussehen:

[NikM]

Das nenne ich mal konstruktive Kritik
Bei der Benennung von Variablen versuche ich mich weitestgehend an die Ungarische Notation zu halten.
Die & und * schreibe ich an den Namen, weil es ja um die Adresse der Variablen geht und nicht um die des Typen oder verwechsel ich da was ?

[Nightblizard]

Die Variable hat z.B. den Typen eines Pointers auf einen int. D.h. * gehört zum Typen. Das Gleiche gilt für Referenzen.

Ich weiß, C++ handhabt das bei der deklaration etwas anders (ein Designfehler imho), aber im moderneren C# z.B. gehört das fest zum Typen.

//C++ Code
int* someInt, someOtherInt; //someInt ist ein zeiger auf einen int, someOtherInt ist ein int. Designfehler.

//C# Code
int* someInt, someOtherInt; //Zwei Zeiger!

Aber im Grunde ist das eine Haarspalterei. Wichtiger ist, dass du Variablennamen generell klein schreibst.

class Foo
{
};

Foo foo;

Code, vorallem wenn du ihn veröffentlichst, sollte so gut wie möglich zu verstehen sein. Die ungarische Notation mag da zwar teilweise helfen, aber sie ist doch ziemlich in die Jahre gekommen.
Dieser Link ist recht nützlich und beschreibt woran man sich derzeit richtet:


Und selbst wenn du dich nicht an alles hälst (ich selber tue das auch nicht in privaten Code), es verbessert die Lesbarkeit doch enorm, wenn man untereinander zu großen Teilen einheitlichen Code schreibt!

[MrSm!th]

Naja, was die kritisierten Konventionen angeht, ist das alles Ansichtssache, da scheiden sich die Geister.

listX ist unnötig. Das müsste der Optimierer zwar entfernen, aber schön ist es deswegen nicht.

Verstehe ich nicht ganz, wieso?

Du solltest die list als const reference übergeben. So wie du das jetzt machst wird eine neue Liste erstellt und der Inhalt aus der einen in die neu erstellte kopiert, wenn du die Funktion aufrufen tust und das ist bei einer Liste mit z.B. 10.000 Einträgen sehr zeitintensiv!

Ich hätte gedacht, die STL Container so arbeiten, dass bei einer Zuweisung (bzw. danach) nur kopiert wird, wenn eine Änderung vorgenommen wird, ansonsten wird dieselbe interne Repräsentation beibehalten.

Die gibt es, um abwärtskompatibel zu sein. Nach heutigen Standards lässt man die Finger von Arrays, wenn es nicht einen sehr guten Grund gibt sie zu nutzen. Selbst Pointer werden inzwischen durch Klassen, die die Ressourcen "überwachen", verdrängt.
Dafür gibt es verdammt viele gute Gründe! Ich will nicht zu weit ins Detail gehen, aber einen will ich dann doch nennen: Sicherheit!
Falls es jemanden interessiert -> googled es. Dazu gibt es viele Themen im Internet, die dieses Thema behandeln.

Mein Reden. Erzähl das bloß nicht Coxxy, der meint, alle Negativargumente gegen C gelten automatisch auch für C++ ;<

btw. @Variablennamen:

Wie schon gesagt, das ist alles Ansichtssache. Es macht zb. Sinn, bestimmten Variablen Präfixe zu geben (nein, ich rede nicht von der unnötigen ungarischen Notation, wodurch dann sowas wie LPCWSTR lpwszStr als Deklaration entsteht), die die Verwendung jener beschreiben.
Eigentlich ist die allgemeine Konvention so, dass lokale Variablen, Hilfsvariablen etc. groß geschrieben werden, globale (sollten sie denn auftreten) und Membervariablen mit entsprechenden Präfixen g_ und m_ gekennzeichnet und danach groß geschrieben werden.
Methoden werden i.d.R. klein geschrieben.

[Nightblizard]

Originally Posted by MrSm!th

Naja, was die kritisierten Konventionen angeht, ist das alles Ansichtssache, da scheiden sich die Geister.

Jau, klar. Das sind ja auch nur Richlinien.

Verstehe ich nicht ganz, wieso?

listX wird in der Funktion LoadList nicht genutzt -> der Parameter ist unnötig.

Ich hätte gedacht, die STL Container so arbeiten, dass bei einer Zuweisung (bzw. danach) nur kopiert wird, wenn eine Änderung vorgenommen wird, ansonsten wird dieselbe interne Repräsentation beibehalten.

Höre ich zum ersten mal und es würde mich wundern, wenn das der Fall wäre.
Doch das bringt mich auf einen anderen Gedanken - der move ctor. Sollte std::list einen solchen haben, dann ist return by value schneller!
Das schaue ich mir später mal an, wenn ich daran denke.

Mein Reden. Erzähl das bloß nicht Coxxy, der meint, alle Negativargumente gegen C gelten automatisch auch für C++ ;<

Das tuen sie auch, wenn du mit C arbeitest.
Gerade deswegen lässt man ja die Finger davon und arbeitet mit Klassen, die bis ins letzte Detail optimiert wurden!

btw. @Variablennamen:

Wie schon gesagt, das ist alles Ansichtssache. Es macht zb. Sinn, bestimmten Variablen Präfixe zu geben (nein, ich rede nicht von der unnötigen ungarischen Notation, wodurch dann sowas wie LPCWSTR lpwszStr als Deklaration entsteht), die die Verwendung jener beschreiben.
Eigentlich ist die allgemeine Konvention so, dass lokale Variablen, Hilfsvariablen etc. groß geschrieben werden, globale (sollten sie denn auftreten) und Membervariablen mit entsprechenden Präfixen g_ und m_ gekennzeichnet und danach groß geschrieben werden.
Methoden werden i.d.R. klein geschrieben.

So spontan kenne ich niemanden, der Variablennamen groß schreibt. Dass globale Variablen (welche man vermeiden soll) g und Klassenmember m als Präfix erhalten ist hingegen durchaus gang und gebe.
An das Kleinschreiben von (öffentlichen) Methoden halte ich mich hingegen wieder nicht, das schaut für mich falsch aus. Private Methoden schreibe ich klein und öffentliche groß.
Wie du schon sagtest, Geschmackssache.