Budowanie klasy Vector w Arduino

Miałeś wiele problemów ze swoim kodem, więc trudno jest wiedzieć, od czego zacząć. 🙂

Zgadzam się z tym, co powiedział John Burger.

Wziąłem twój kod na swój komputer, aby uniknąć bałaganu przy przesyłaniu go za każdym razem, a także aby móc użyć na nim valgrind. Z pewnością Valgrind zgłosił błąd po wydrukowaniu wektora. Powód jest prosty. Przekazałeś kopiowanie klasy do print_vec, co oznacza, że destruktor uzyskał wywoływana po zakończeniu działania print_vec, zwalniając tym samym pamięć. Powinieneś mieć konstruktora kopiującego . Bez tego kompilator tworzy bitową kopię klasy, co oznacza, że masz teraz dwa obiekty współużytkujące tę samą przydzieloną pamięć.

Szybkim i brudnym rozwiązaniem jest wywołanie print_vec przez odniesienie:

void print_vec(vector<int> & v){ 

Jednak ten błąd czai się na przyszłość.

Zaimplementowałem konstruktor kopiujący w moim przykładzie poniżej, jednak wywołanie print_vec przez odniesienie oszczędza konieczność skopiowania klasy, zmniejszając liczbę new/delete wykonywanych czynności, co prawdopodobnie zmniejsza fragmentacja pamięci.

Jak powiedział John Burger: nie wzywaj sam destruktora! Ty nie można tego zrobić. Jeśli chcesz zrobić to samo w destruktorze i funkcji clear, po prostu poproś go o wywołanie funkcji clear().

Stosowanie początkowych podwójnych podkreśleń w zmiennych jest sprzeczne ze standardem C ++. Nie rób tego. Użyj końcowego podkreślenia, jeśli chcesz wskazać zmienną składową. Strać wszystkie odniesienia this->. Po prostu zaśmiecają rzeczy.

Ponieważ przydzielasz tablicę, powinieneś użyć delete [] – zrobiłeś to w jednym miejscu, ale nie w drugim.

Dlaczego to robisz? To niepotrzebne przypisanie:

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

Po prostu zrób:

 vector<int> i; 

Jeśli zamierzasz przypisać klasę z jakiegoś powodu, powinieneś także zaimplementować operator= lub będziesz mieć te same problemy, co z konstruktorem kopiującym. (Zobacz mój przykład).

Tutaj testujesz funkcje (pojemność i rozmiar) bez ich wywoływania:

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

Naprawdę nie możesz tego zrobić :

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

Jeśli wybierzesz ścieżkę „else”, funkcja nie zwraca żadnej wartości (jeśli włączysz ostrzeżenia, otrzymasz ostrzeżenie kompilatora).

Mój przerobiony przykład z moimi sugestiami. Kompiluje się bez ostrzeżeń i błędów i działa bez awarii (na komputerze 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(); } } 

Komentarze

• W porządku, wielkie dzięki! Wiele problemów było po prostu rozproszeniem po mojej stronie, próbowałem skupić się na naprawieniu problemów z pamięcią, z którymi miałem do czynienia na początku.

W swoim drugim konstruktorze zaczynasz od następującego kodu:

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

To jest śmiertelne! Nie zainicjowałeś __data, więc może zawierać dowolną wartość pod słońcem. I nie ma sposobu , aby mógł prawdopodobnie być prawidłowym wskaźnikiem do istniejących danych – to zupełnie nowy niezainicjowany obiekt. Zwrócenie tego wskaźnika śmieci na stertę to po prostu prośba o kłopoty.

Usuń te linie – lub nawet lepiej, użyj listy inicjalizatorów, tak jak w przypadku pierwszego konstruktora:

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) 

Inny problem: w Twój clear() członek, którego napisałeś:

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

Nigdy, nigdy , nie powinieneś, nigdy bezpośrednio wywołuj taki destruktor. Kompilator może umieścić w kodzie destruktora wszelkiego rodzaju inny kod, ponieważ „wie”, że jest to jedyny, który kiedykolwiek go wywoła. Na przykład niektóre kompilatory wypychają flagę, aby powiedzieć, czy delete obiekt ze sterty po zniszczeniu. Nie zrobiłeś tego, więc powyższe może uszkodzić różne rzeczy.

Zamiast tego przenieś kod, który jest w Destruktor do clear(), a następnie po prostu wywołaj clear() z destruktora.

Kolejny, ten odebrany autor: @Nick Gammon:

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

Sprawdzam, czy element członkowski __data jest fałszywy i czy element capacity i size zostały zdefiniowane. Nie martw się: dwa ostatnie zostały … przegapiłeś prefiks __ …

[Poza tym zakończ go this-> wszędzie. Użyłeś __ przed wszystkimi zmiennymi składowymi (co samo w sobie jest sprzeczne z konwencją ); nie musisz tłumaczyć faktu, że są to zmienne składowe: kompilator już wie.]

Komentarze

• Usunąłem cały konstruktor vector(T obj, int count = 1) i skompilowałem go bez błędów, więc nie było ' t problem w tym przypadek, ale masz rację, że __data nie został zainicjowany.
• should never have compiled – testował funkcje, a nie wywoływał je. capacity jest funkcją składową, a nie zmienną. Podniosłem to, kiedy próbowałem skompilować ją pod clang.
• Hej hee! Przegapiłem to – a Ty ' masz rację!
• Mała rada: Twój kod jest praktycznie nieczytelny. Użyj lepszego stylu kodowania : google.github.io/styleguide/cppguide.html
• Nie rozumiem też, dlaczego to robisz. Chociaż w pełni STL nie jest obsługiwany, istnieje kilka dobrze debugowanych plików ed implementacje, które można znaleźć za pomocą prostego wyszukiwania w Google. Jeśli robisz to, aby nauczyć się C ++, środowisko arduino jest prawdopodobnie najgorszym sposobem na zrobienie tego!

Chciałbym coś dodać.
Inni już wskazali liczne błędy w kodzie. Mógłbym wskazać znacznie więcej, ale nie o to chodzi.
IMO, największym błędem jest – implementacja klasy Vector dla Arduino!
Nawet jeśli to działa, to po prostu zły pomysł. Należy pamiętać, że Arduino ma BARDZO ograniczoną ilość pamięci. Dynamiczna alokacja pamięci w tak ograniczonej przestrzeni po prostu brzmi jak zły pomysł – marnujesz pamięć, ponownie fragmentujesz stertę i prawie nic nie zyskujesz, poza pewną wygodą kodowania.
Nie widziałem jeszcze przypadku, w którym tak naprawdę potrzebuje dynamicznej alokacji pamięci w projekcie Arduino. A nawet jeśli to zrobisz, istnieje wiele innych narzędzi, których możesz użyć (np. bufory tymczasowe przydzielane stosowi, alokatory areny, alokatory puli itp.). Ale znowu, prawdopodobnie kod i koszt pamięci te „ogólne” rozwiązania będą niewykonalne na Arduino.
W większości przypadków dobrze jest używać wektorów, map i wszystkiego, co lubisz na platformach takich jak PC (pracuję jako programista gier na PC, więc często napotykam sytuacje gdzie dodatki takie jak te są duże „nie, nie”, ale w przypadku większości zastosowań jest w porządku).
Ale na „ciasnej” platformie, takiej jak Arduino, uważam, że należy pozostać „blisko metalu” i w całkowitej kontroli nad tym, co się dzieje (pod względem pamięci i cykli procesora) .Najlepiej byłoby pozwolić sobie tylko na luksus es „jak te, kiedy wiesz, co się dzieje” pod spodem „i wiesz, że ujdzie ci to na sucho. Co rzadko się zdarza na „ciasnych” platformach, takich jak Arduino.

Komentarze

• Masz rację. W szczególności pomysł push_back, który przyrostowo przydziela więcej pamięci (co zwykle jest dobre na komputerze z 4 GB pamięci RAM), prawdopodobnie spowoduje fragmentację pamięci. Myślę, że nawet normalne implementacje STL alokują pamięć wektorową (pojemność w tym przypadku) partiami (np. 10 na raz), aby zmniejszyć ten problem.