Het bouwen van een Vector-klasse in Arduino

Je had veel problemen met je code, dus het is moeilijk om te weten waar je moet beginnen. 🙂

Ik ben het eens met wat John Burger zei.

Ik heb je code op mijn pc gezet om te voorkomen dat ik het elke keer opnieuw upload, en ook zodat ik er valgrind op kon gebruiken. Valgrind heeft zeker een fout gerapporteerd nadat u de vector had afgedrukt. De reden daarvoor is simpel. Je hebt een kopie van de klas doorgegeven aan print_vec wat betekende dat de vernietiger aangeroepen wanneer print_vec werd afgesloten, waardoor de toewijzing van het geheugen ongedaan wordt gemaakt. Je zou een copy constructor moeten hebben. Zonder dit maakt de compiler een bitsgewijze kopie van de klasse, wat betekent dat je nu twee objecten hebt die hetzelfde toegewezen geheugen delen.

Een snelle en vuile oplossing is om print_vec via referentie:

void print_vec(vector<int> & v){ 

Maar dat laat de bug op de loer liggen voor de toekomst.

Ik heb de kopieerconstructor geïmplementeerd in mijn voorbeeld hieronder, maar als u print_vec aanroept door middel van verwijzing, hoeft de klasse niet te worden gekopieerd, waardoor het aantal new/delete s dat u aan het doen bent, wordt verminderd, waardoor u mogelijk minder geheugenfragmentatie.

Zoals John Burger zei: noem de destructor niet zelf! Jij kan dat niet doen. Als u hetzelfde wilt doen in de destructor en de functie clear, laat dan de destructor clear().

Het gebruik van vooraanstaande dubbele onderstrepingstekens in variabelen is in strijd met de C ++ -standaard. Doe dat niet. Gebruik een onderstrepingsteken als u een lidvariabele wilt aangeven. Verlies alle this-> referenties. Ze maken de zaken gewoon rommelig.

Aangezien je een array toewijst, moet je delete [] gebruiken – je deed dat op de ene plaats maar niet op de andere.

Waarom doe je dit? Het is een onnodige opdracht:

 vector<int> i = vector<int>(); 

Gewoon doen:

 vector<int> i; 

Als je de klasse gaat toewijzen om de een of andere reden moet je ook een operator= implementeren, anders heb je dezelfde problemen als met de kopieerconstructor. (Zie mijn voorbeeld).

Hier test je functies (capaciteit en grootte) zonder ze aan te roepen:

bool isempty() { return !this->__data || !this->capacity || !this->size; } 

Je kunt dit “niet echt doen :

 T const & operator[] (unsigned int idx) const { if (idx < this->__count) { return this->__data[idx]; } else { // throw exception or handle error to be implemented } } 

Als je het “else” -pad volgt, retourneert de functie geen waarde (je krijgt een compilerwaarschuwing als je waarschuwingen inschakelt).

Mijn herwerkte voorbeeld, met mijn suggesties erin. Het compileert zonder waarschuwingen of fouten en werkt zonder te crashen (op een pc):

// g++ -std=c++11 -g3 test.cpp -o test // OR: clang -std=c++11 -g3 -Wall test.cpp -lstdc++ -o test // // valgrind --leak-check=yes ./test #include <stdio.h> // printf #include <string.h> /* memcpy */ #define min(a,b) (a < b) ? a : b template <typename T> class vector { public: //empty constructor vector() : capacity_(0), count_(0), data_(nullptr) {} // copy constructor vector (const vector & rhs) : capacity_(0), count_(0), data_(nullptr) { data_ = new T [rhs.capacity_]; capacity_ = rhs.capacity_; count_ = rhs.count_; memcpy (data_, rhs.data_, sizeof (T) * count_); } // end of copy constructor // operator= vector & operator= (const vector & rhs) { // gracefully handle self-assignment (eg. a = a;) if (this == &rhs ) return *this; data_ = new T [rhs.capacity_]; capacity_ = rhs.capacity_; count_ = rhs.count_; memcpy (data_, rhs.data_, sizeof (T) * count_); return *this; } // end of operator= // destructor ~vector() { clear (); } // end of destructor T const & operator[] (unsigned int idx) const { return data_[idx]; } T& operator[] (unsigned int idx) { return data_[idx]; } void resize_to_fit() { resize(count_); } T& pop_back(){ return data_[--count_]; } void push_back(T obj) { if (capacity_ == count_) { resize(capacity_ + 1); } data_[count_++] = obj; } bool isempty() { return count_ == 0; } void clear() { delete [] data_; data_ = nullptr; count_ = capacity_ = 0; } T* data() { return data_; } int size() { return count_; } int capacity() { return capacity_; } private: void resize(int capacity) { if (data_ == nullptr) { data_ = new T[capacity]; count_ = 0; capacity_ = capacity; } else if (capacity > capacity_) { //else do nothing // allocate new memory T* data = new T[capacity]; // count can"t be higher than capacity count_ = min(count_, capacity); capacity_ = capacity; // copy data across to new pointer memcpy(data, data_, sizeof(T) * count_); // delete old pointer delete [] data_; // now remember the new pointer data_ = data; } } protected: int capacity_; int count_; T* data_; }; void print_vec(vector<int> v){ printf("%s\n", "Printing new vec!"); for( int i = 0; i < v.size(); i++){ printf("%i\n", v[i]); } } int main() { vector<int> i; i.push_back(10); i.push_back(2); print_vec(i); i.push_back(3); vector<int> j; // test assignment j = i; print_vec(j); print_vec(i); while(i.size () > 0){ print_vec(i); i.pop_back(); } } 

Reacties

• Oké, heel erg bedankt! Veel van de problemen waren gewoon afleiding aan mijn kant, ik probeerde me te concentreren op het oplossen van de geheugenproblemen waarmee ik in eerste instantie werd geconfronteerd.

In je tweede constructor begin je met de volgende code:

if (this->__data) { // free data ptr if used delete this->__data; } // if 

Dit is dodelijk! U “heeft __data niet geïnitialiseerd, dus het zou elke waarde onder de zon kunnen bevatten. En er is geen enkele manier dat het zou kunnen mogelijk een geldige verwijzing zijn naar bestaande gegevens – het is een gloednieuw niet-geïnitialiseerd object. Het terugbrengen van deze vuilnisaanwijzer naar de hoop is gewoon vragen om problemen.

Verwijder die regels – of nog beter, gebruik een initialisatielijst zoals je deed met de eerste constructor:

vector(T obj, int count = 1) : __data(new T[count]), __count(count), __capacity(count) { for (int i = 0; i < count; i++) //populate array with given object __data[i] = obj; } // vector(T, count) 

Een ander probleem: In uw clear() lid dat u schreef:

void clear() { this->~vector(); } 

Je mag nooit, nooit , noem nooit rechtstreeks een destructor op deze manier. De compiler kan allerlei andere code in de destructor-code stoppen, omdat hij “weet” dat dit de enige is die hem ooit zal aanroepen. Sommige compilers drukken bijvoorbeeld een vlag vooraf om te zeggen of delete het object van de hoop nadat je de vernietiging hebt uitgevoerd. Je hebt dat niet gedaan, dus het bovenstaande kan allerlei dingen beschadigen.

Verplaats in plaats daarvan de code die in de destructor in clear(), en dan gewoon clear() aanroepen vanaf de destructor.

Nog een, deze heeft opgepikt door @Nick Gammon:

bool isempty() { return !this->__data || !this->capacity || !this->size; } 

Hiermee wordt getest of het __data -lid onwaar is en of het capacity en size functies zijn gedefinieerd. Maak je geen zorgen: de laatste twee zijn … je hebt het voorvoegsel __ …

[Stop het ook met de this-> overal. Je “hebt een __ gebruikt voor al je lidvariabelen (wat op zichzelf in strijd is met ); je hoeft niet te hameren op het feit dat het lidvariabelen zijn: de compiler weet het al.]

Reacties

• Ik heb de volledige vector(T obj, int count = 1) constructor verwijderd en deze zonder fouten gecompileerd, dus dat was niet ‘ het probleem in deze case, maar je hebt gelijk dat __data niet werd geïnitialiseerd.
• should never have compiled – het testte de functies in plaats van aan te roepen ze. capacity is een lidfunctie, geen variabele. Ik heb dat opgemerkt toen ik het probeerde te compileren onder clang.
• Hee hee! Ik heb dat gemist – en jij ‘ hebt gelijk!
• een beetje advies: je code is praktisch onleesbaar. Gebruik een betere codeerstijl : google.github.io/styleguide/cppguide.html
• Ik begrijp ook niet waarom je dit doet. Hoewel volledig STL wordt niet ondersteund, er zijn verschillende goed-debugg ed implementaties die u kunt vinden met een eenvoudige Google-zoekopdracht. Als je het doet om C ++ te leren, is de arduino-omgeving waarschijnlijk de slechtste manier om het te doen!

Er is iets dat ik zou willen toevoegen.
Anderen hebben al op talloze fouten in de code gewezen. En ik zou er nog veel meer kunnen noemen, maar daar gaat het niet om. IMO, de grootste fout is – het implementeren van een Vector-klasse voor de Arduino!
Zelfs als het werkt, is het gewoon een slecht idee. Houd er rekening mee dat de Arduino een ZEER beperkte hoeveelheid geheugen heeft. Dynamische geheugentoewijzing in zon beperkte ruimte klinkt gewoon als een slecht idee – je verspilt geheugen, je fragmenteert de hoop en je wint bijna niets, behalve wat codeergemak.
Ik moet nog een geval zien waarin een echt heeft dynamische geheugentoewijzing nodig in een Arduino-project. En zelfs als je dat doet, zijn er nog veel andere tools die je kunt gebruiken (zoals stack-toegewezen tijdelijke buffers, arena-allocators, pool-allocators en wat niet). Maar nogmaals, waarschijnlijk de code en geheugenkosten van deze “generieke” oplossingen zullen onhaalbaar zijn op de Arduino.
Meestal is het prima om vectoren, kaarten en wat je maar wilt te gebruiken op platforms zoals pc (ik werk als programmeur van pc-games, dus ik kom vaak situaties tegen waar dit soort extras groot “nee-nee” zijn, maar voor de meeste toepassingen is het prima).
Maar op een “strak” platform zoals de Arduino, denk ik dat je “dicht bij het metaal” moet blijven en in totale controle over wat er gaande is (geheugen- en CPU-cycli verstandig). Idealiter zou u zich alleen “luxuri es “zoals deze als je weet wat er” eronder gebeurt “en je weet dat je ermee weg kunt komen. Dat is zelden het geval op “strakke” platforms zoals de Arduino.

Reacties

• Je hebt hier een punt. Met name het push_back idee dat stapsgewijs meer geheugen toewijst (wat normaal gesproken prima is op een pc met 4 Gb RAM) zal waarschijnlijk geheugenfragmentatie introduceren. Ik denk dat zelfs normale STL-implementaties vectorgeheugen (capaciteit in dit geval) in batches toewijzen (bijv. 10 tegelijk) om dit probleem te verminderen.