Quote:
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.
Code:
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.
Code:
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.
Code:
if (Output != NULL) //Nur wenn ein File erstellt wurde
Hier solltest du "Output.is_open()" nutzen, da es einfach aussagekräftiger ist.
Code:
{
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! ;)
Code:
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! ;)
Code:
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:
[Only registered and activated users can see links. Click Here To Register...]