Was haltet ihr von meiner eigenen CONTAINER Klasse?
-
Shade Of Mine schrieb:
dir ist schon klar dass es gängig ist auf der höchsten optimierungsstufe iteratoren in vectoren als reine zeiger zu implementieren, oder?
Ich wollte nur das potenzielle Missverständnis vermeiden, dass Iteratoren generell immer nur Zeiger (mit deren Arithmetik) wären - auch wenn es bei
std::vectorso sein mag, ist es nicht bei allen Containern der Fall. Ich bin mir auch bewusst, dass es hier um ein vector-ähnliches Gebilde geht. Aber demnach ist der Bezeichneriteratorhier durchaus gerechtfertigt...
-
Also ich hab das ganze noch mal ein bischen überarbeite
Ich habe an einigen Stellen auf eure Tipps bzw. Kritik gehört und hoffe dass meien Klasse jetzt schon eher ansprechend aussieht
Erneuerungen:
(* ein paar Namensänderungen)
* delete [] data_
* "std::copy" & "std::fill" statt "memcpy" & "memset"
* zusätzlich: const T& operator [] (u32 index) const { /* ... */ }
* kein NULL mehr in Benutzung
* "push_back" & "push_front" statt "add"
* eigene 'typedefs' wie "u32" statt "unsigned int"Hier ist die neue Klasse:
// // ============ math -> Container class ============ // // Copyright (c) 2008 Lukas Hermanns // #ifndef __MATH_CONTAINER__ #define __MATH_CONTAINER__ #include <algorithm> #include <cstdlib> #include <math.h> typedef char c8; typedef wchar_t c16; typedef signed char s8; typedef signed short s16; typedef signed int s32; typedef signed long int s64; typedef unsigned char u8; typedef unsigned short u16; typedef unsigned int u32; typedef unsigned long int u64; typedef float f32; typedef double f64; namespace dim { template <typename T> class array { public: /* Type definitions */ typedef T* iterator; typedef T* const const_iterator; /* Constructors */ array() : data_(0), len_(0) { } array(u32 size) : data_(0), len_(0) { create(size); } /* Destructor */ ~array() { clear(); } /* Operators - comparision */ bool operator == (array<T> other) { /* Check if the arrays are not emtpy */ if (!data_ || !other.data_) return false; /* Loop the elements */ for (u32 i = 0; i < len_; ++i) { if (data_[i] != other.data_[i]) return false; } /* Exit the function */ return true; } bool operator != (array<T> other) { /* Check if the arrays are not emtpy */ if (!data_ || !other.data_) return false; /* Loop the elements */ for (u32 i = 0; i < len_; ++i) { if (data_[i] != other.data_[i]) return true; } /* Exit the function */ return false; } /* Operators - settings / gettings */ T& operator [] (u32 index) { return data_[index]; } const T& operator [] (u32 index) const { return data_[index]; } array<T>& operator = (const array<T> &other) { /* Delete the old memory */ if (data_) delete [] data_; /* Allcoate new memory */ data_ = new T[other.len_]; len_ = other.len_; /* Copy the memory */ std::copy(&other.data_[0], &other.data_[other.len_], &data_[0]); /* Return the reference of this object */ return *this; } /* Functions */ bool empty() { return !data_; } u32 size() { return len_; } const_iterator begin() { return data_ ? &data_[0] : 0; } const_iterator end() { return data_ ? &data_[len_] : 0; } void create(u32 size) { /* Delete the old memory */ if (data_) delete [] data_; /* Allocate new memory */ data_ = new T[size]; /* Set the length of the memory */ len_ = size; /* Initialize the new memory */ std::fill(begin(), end(), (T)0); } void clear() { if (data_) { delete [] data_; data_ = 0; } len_ = 0; } void push_back(const T &obj) { /* Allocate a temporary memory */ T* tmp = new T[len_ + 1]; /* Copy the memory */ std::copy(begin(), end(), &tmp[0]); /* Add the new object */ tmp[len_] = obj; /* Delete the old memory */ if (data_) delete [] data_; /* Use the new memory */ data_ = tmp; ++len_; } void push_front(const T &obj) { /* Allocate a temporary memory */ T* tmp = new T[len_ + 1]; /* Copy the memory */ ++tmp; std::copy(begin(), end(), &tmp[0]); --tmp; /* Add the new object */ tmp[0] = obj; /* Delete the old memory */ if (data_) delete [] data_; /* Use the new memory */ data_ = tmp; ++len_; } void push_back(const T array[], u32 size) { for (register u32 i = 0; i < size; ++i) push_back(array[i]); } void push_front(const T array[], u32 size) { for (register u32 i = 0; i < size; ++i) push_front(array[i]); } void insert(const_iterator it, const T &obj) { /* Allocate a temporary memory */ T* tmp = new T[len_ + 1]; /* Copy the memory */ std::copy(begin(), end(), &tmp[0]); /* Loop the old memory */ for (register u32 i = len_; i > it - &data_[0]; --i) tmp[i] = data_[i-1]; /* Add the new object */ tmp[it - &data_[0]] = obj; /* Delete the old memory */ if (data_) delete [] data_; /* Use the new memory */ data_ = tmp; ++len_; } void erase(const_iterator it) { /* Allocate a temporary memory */ T* tmp = new T[len_ - 1]; /* Copy the memory */ std::copy(begin(), end() - 1, &tmp[0]); /* Loop the old memory */ for (register u32 i = it - &data_[0]; i < len_ - 1; ++i) tmp[i] = data_[i+1]; /* Delete the old memory */ if (data_) delete [] data_; /* Use the new memory */ data_ = tmp; --len_; } void erase(const_iterator begin, const_iterator end) { /* Check if the data is not empty */ if (!data_) return; /* Allocate a temporary memory */ const u32 less = end - begin + 1; T* tmp = new T[len_ - less]; /* Copy the memory */ std::copy(begin(), end() - less, &tmp[0]); /* Loop the old memory */ for (register u32 i = begin - &data_[0]; i < len_ - less; ++i) tmp[i] = data_[i+less]; /* Delete the old memory */ delete [] data_; /* Use the new memory */ data_ = tmp; len_ -= less; } void erase_front() { /* Check if the data is not empty */ if (!data_) return; /* Allocate a temporary memory */ T* tmp = new T[len_ - 1]; /* Save the old mamory */ T* oldarray = data_; /* Move the old memory */ ++data_; /* Copy the memory */ std::copy(begin(), end() - 1, &tmp[0]); /* Delete the old memory */ delete [] oldarray; /* Use the new memory */ data_ = tmp; --len_; } void erase_back() { /* Check if the data is not empty */ if (!data_) return; /* Allocate a temporary memory */ T* tmp = new T[len_ - 1]; /* Copy the memory */ std::copy(begin(), end() - 1, &tmp[0]); /* Delete the old memory */ delete [] data_; /* Use the new memory */ data_ = tmp; --len_; } void move(const_iterator itbegin, const_iterator itend, const_iterator it) { /* Check if the data is not empty */ if (!data_) return; /* Allocate a temporary memory */ T* tmp = new T[len_]; /* Copy the memory */ std::copy(begin(), end(), &tmp[0]); /* Move the memory */ register u32 i; for (i = 0; i <= itend - itbegin; ++i) data_[i + (it - &data_[0])] = tmp[i + (itbegin - &data_[0])]; /* Move the rest of the memory */ if (it <= itbegin) { for (i = 0; i < itbegin - it; ++i) data_[i + (it - &data_[0]) + itend - itbegin + 1] = tmp[i + (it - &data_[0])]; } else { for (i = 0; i < it - itbegin; ++i) data_[i + (itbegin - &data_[0])] = tmp[i + (itend - &data_[0] + 1)]; } /* Delete temporary meory */ delete [] tmp; } void resize(u32 newsize) { /* Check if the data is not empty */ if (!data_) return; /* Allocate a temporary memory */ T* tmp = new T[newsize]; /* Initialize the new memory */ std::fill(&tmp[0], &tmp[newsize], (T)0); /* Copy the memory */ const_iterator itend = (newsize > len_ ? end() : begin() + newsize); std::copy(begin(), itend, &tmp[0]); /* Delete the old memory */ delete [] data_; /* Use the new memory */ data_ = tmp; len_ = newsize; } void swap(array<T> &other) { /* Check if the data are not emtpy */ if (!data_ && !other.data_) return; /* Allocate temporary memory */ T* tmp = data_; u32 len = len_; /* Swap the memories */ data_ = other.data_; other.data_ = tmp; /* Swap the lengths */ len_ = other.len_; other.len_ = len; } void flip() { /* Check if the data is not empty */ if (!data_) return; /* Allocate temporary memory */ T* tmp = new T[len_]; /* Fill the temporary memory */ std::copy(begin(), end(), &tmp[0]); /* Loop and flip the array */ for (register u32 i = 0; i < len_; ++i) data_[i] = tmp[len_ - i - 1]; /* Delete the temorary memory */ delete [] tmp; } void unique() { /* Loop the data - main */ for (register u32 i = 0, j; i < len_; ++i) { /* Loop the data - sub */ for (j = 0; j < len_;) { if (i != j && data_[i] == data_[j]) erase(begin() + j); else ++j; } } } void sort() { /* Check if the data is not empty */ if (!data_) return; /* Loop the data - main */ for (register u32 i = 0, j; i < len_; ++i) { /* Loop the data - sub */ for (j = 0; j < len_; ++j) { /* Check which value is smaller */ if (data_[i] < data_[j] && i != j) { /* Then move it before the element with the bigger value */ move(begin()+i, begin()+i, begin()+j); } } } } private: /* Members */ T* data_; u32 len_; }; } // /namespace math #endif
-
typedef T* const const_iterator;Argh! Ich weiß nicht, ob du das Konzept von Iteratoren schon verstanden hast. Lies mal das.
EDIT:
Ich mach dir mal ne Liste von WTFs in deinem Code. Es besteht allerdings keine Grantie auf Vollständigkeit.1.) #ifndef __MATH_CONTAINER__ 2.) #include <math.h> 3.) Wozu die typedefs? Wenn dann in der Klasse selbst. 4.) typedef T* const const_iterator; 5.) bool operator == (array<T> other) // Deklaration + Definition 6.) bool operator != (array<T> other) // Deklaration + Definition 7.) array<T>& operator = (const array<T> &other) // Selbstzuweisung? 8.) bool empty() // Deklaration 9.) u32 size() // Deklaration 10.) const_iterator begin() // Deklaration, einzige Version? 11.) const_iterator end() // Deklaration, einzige Version? 12.) std::fill(begin(), end(), (T)0); // Beachte, dass begin() + end() bei dir eig. const_iteratoren sind, (T)0 ? User defined types? 13.) if (data_) delete [] data_; // doppelt-gemoppelt hält besser, was? 14.) void erase(const_iterator begin, const_iterator end) // Folge-Fehler 15.) void swap(array<T> &other) // Viel zu umständlich, könnte ein Zwei-Zeiler sein.
-
Nexus schrieb:
MontagMorgen schrieb:
class math : public std::vector<T>{};Es gab hier schon mehrere Diskussionen darüber, dass man nicht von STL-Containern erben sollte (u.a. wegen nicht vorhandenem virtuellen Destruktor).
Das sollte meines Erachtens gerade ein Beispiel sein, in dem man es darf (weil die abgeleitete Klasse keine Membervariablen hat und ihr Destruktor damit trivial ist). Damit sie benutzbar ist, sollten aber noch die Konstruktoren implementiert sein.
-
Nimm mal deinen RL-Namen aus dem Sourcecode hier raus
Reale Existenzen haben im Internet nix zu suchen 
-
.filmor schrieb:
Das sollte meines Erachtens gerade ein Beispiel sein, in dem man es darf (weil die abgeleitete Klasse keine Membervariablen hat und ihr Destruktor damit trivial ist).
Das konstituiert keinen trivialen Destruktor. Ein trivialer Destruktor ist an dieser Stelle ohnehin kein Kriterium.
-
Sondern?
-
[unsinn]
-
-
zeusosc schrieb:
array<T>& operator = (const array<T> &other) // Selbstzuweisung?alternative??
Grüüüße
Don06 meinte damit wohl das eine mögliche Selbstzuweisung aktuell nicht behandelt wird.
Edit:
Was mir nach auffielo array(u32 size) sollte explizit sein
o Vergleichsoperatoren halte ich für unnötig
o Die Const-Correctness wurde nicht konsequent eingehalten (z.B. size())
o Die "Wachstumsstrategie" ist weniger optimal (viel zu viel Allokationen)
o Kein Kopierkonstruktor
o Kein korrektes Exceptionhandling, gerade bei Neuanforderung von Speicher. Das kann schnell zu ungültigen Zeigern und haufwenweisen Problemen führen.
-
Abgesehn von der Definition des "iterator"s und des "const_iterator"s sieht das jetzt aber schon relativ gut aus, oder?!
-
David_pb schrieb:
zeusosc schrieb:
array<T>& operator = (const array<T> &other) // Selbstzuweisung?alternative??
Grüüüße
Don06 meinte damit wohl das eine mögliche Selbstzuweisung aktuell nicht behandelt wird.
Edit:
Was mir nach auffielo array(u32 size) sollte explizit sein
o Vergleichsoperatoren halte ich für unnötig
o Die Const-Correctness wurde nicht konsequent eingehalten (z.B. size())
o Die "Wachstumsstrategie" ist weniger optimal (viel zu viel Allokationen)
o Kein Kopierkonstruktor
o Kein korrektes Exceptionhandling, gerade bei Neuanforderung von Speicher. Das kann schnell zu ungültigen Zeigern und haufwenweisen Problemen führen.ok,..
Wie behandelt man korrekterweise eine Selbstzuweisung??
Ich benutze eine ähnliche "Wachsutmsstrategie". Wie kann man das vermeiden?? (ausser nutzung std::container á la vector)
Wie sieht eine korrekte Fehlerbehandlung bei neuanforderung von speicher aus, ausser das das System sagt "hmmm, nö!".
*abo*
grüüße
-
zeusosc schrieb:
Wie behandelt man korrekterweise eine Selbstzuweisung??
Die schlechte Methode:
if(this == &other)
die gute Methode:
operator= mit Hilfe des CopyCtors so implementieren:T& operator=(T const& other) { T temp(other); swap(temp); return *this; }Ich benutze eine ähnliche "Wachsutmsstrategie". Wie kann man das vermeiden?? (ausser nutzung std::container á la vector)
Standardmaessig nimmt man mal 1,3 oder 1,5 wenn man aggresiv vorgeht mal 2.
Wie sieht eine korrekte Fehlerbehandlung bei neuanforderung von speicher aus, ausser das das System sagt "hmmm, nö!".
du wirfst eine exception und sagst: sorry, kann den container nicht vergroessern - aber der container bleibt dennoch in einem ordentlichen zustand, lediglich die einfuege operation ist fehlgeschlagen.
-
LukasBanana schrieb:
/* Operators - comparision */ bool operator == (array<T> other) { /* Check if the arrays are not emtpy */ if (!data_ || !other.data_) return false; ... bool operator != (array<T> other) { /* Check if the arrays are not emtpy */ if (!data_ || !other.data_) return false;Zweimal die gleiche Bedingung mit dem selben Rückgabewert in gleich und ungleich?
Die selben beiden Container können also gleichzeitig weder gleich noch ungleich sein. 
-
Shade Of Mine schrieb:
Die schlechte Methode:
if(this == &other)
Weshalb ist diese Methode schlechter? Weil man mehr Code schreiben muss, anstatt bereits Existierendes (Kopierkonstruktor) wiederzuverwenden?
Antilogik schrieb:
Zweimal die gleiche Bedingung mit dem selben Rückgabewert in gleich und ungleich?
Die selben beiden Container können also gleichzeitig weder gleich noch ungleich sein. Da könnte man auch den einen Operator durch den (negierten) anderen ausdrücken.
-
Nexus schrieb:
Shade Of Mine schrieb:
Die schlechte Methode:
if(this == &other)
Weshalb ist diese Methode schlechter? Weil man mehr Code schreiben muss, anstatt bereits Existierendes (Kopierkonstruktor) wiederzuverwenden?
-
Nach deinem Link sind Argumente gegen
if (this != &other)eine überflüssige Prüfung auf Zuweisung bei exception-sicherer Umgebung und eine Gefahr durch überladenenoperator&, soweit ich das verstanden habe. Aber wird der Adressoperator in der Praxis häufig überladen?Und bei der anderen Methode (Copy and Swap) wird immerhin ein temporäres Objekt erzeugt, ist das nicht langsamer? Oder wird das wegoptimiert?
-
Ich bezweifle stark, dass sich bei der «besseren» Methode die Temporäre Kopie vermeiden lässt, da jene Kopie an sich diese Methode «sicherer» macht. Jedenfalls habe ich das so verstanden.
Kommt allerdings mehr als nur etwas auf die Implementierung von swap() an wie sehr das ein Problem ist. Bei einer std::list z.B. stell' ich mir das wenig problematisch vor.
-
Um das Thema dieses Threads nicht weiter zu behindern, habe ich die Diskussion in einen eigenen Thread ausgelagert.
-
darthdespotism schrieb:
Nexus schrieb:
Shade Of Mine schrieb:
Die schlechte Methode:
if(this == &other)
Weshalb ist diese Methode schlechter? Weil man mehr Code schreiben muss, anstatt bereits Existierendes (Kopierkonstruktor) wiederzuverwenden?
was steht da??
