Vector class for Arduino

[R3DDOT]

I'm struggling with this issue that I can seem to get around of.
When defining a vector class, I'm seeming to get some issues when deleting the allocated pointer.

Oddly enough, this only happens after I "read" the memory. What I mean by this is that if I push back a series of values to the vector, they seem to stay there. But once I read the values, the data pointer seems to become invalid of some sort so I crash when i try to deallocate it.

#ifndef VECTOR_H
#define VECTOR_H

#include <string.h>		/* memcpy */

#define min(a,b) (a < b) ? a : b

template <typename T>
class vector {
public:
	vector() : __capacity(0), __count(0), __data(0) {} //empty constructor
	vector(T obj, int count = 1) {

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

		this->__data = new T[count];
		this->__count = count;
		this->__capacity = count;

		for (int i = 0; i < count; i++) //populate array with given object
			this->__data[i] = obj;

	}

	~vector() {

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

		this->__count = this->__capacity = 0;
	}

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

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

	void resize_to_fit() {
		resize(this->__count);
	}

	T& pop_back(){
		return this->__data[--this->__count];
	}

	void push_back(T obj) {

		if (this->__capacity == this->__count) {
			resize(this->__capacity + 1);
		}

		this->__data[this->__count++] = obj;
	}

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

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

	T* data() {
		return this->__data;
	}

	int size() {
		return this->__count;
	}

	int capacity() {
		return this->__capacity;
	}
private:
	void resize(int capacity) {
		if (this->__data == nullptr) {
			this->__data = new T[capacity];

			this->__count = 0;
			this->__capacity = capacity;
		}
		else if (capacity != this->__capacity) { //else do nothing

			T* data = new T[capacity];

			this->__count = min(this->__count, capacity);
			this->__capacity = capacity;

			memcpy(data, this->__data, sizeof(T) * this->__count);

			delete this->__data; //program crashes here, but the pointer is already broken...
			this->__data = data;
		}
	}

private:

	int __capacity;
	int __count;
	T* __data;

};
*/
#endif//VECTOR_H

I ran it on this console:

int main(void) {

	vector<int> v = vector<int>(10); //Works fine, created the vector

	v.push_back(20); //still fine here
        v.push_back(30); //still fine here
	print_vec(v); //shows 10, 20,30 so working just fine so far

	v.push_back(10); //crashes here

	print_vec(v);

	v[1] = 10;

	print_vec(v);

	v.pop_back();
	print_vec(v);

	Sleep(10000);
}

[pushedx]

There's a lot of C++ nuances you need to be aware of to understand why your code is exhibiting the behavior it currently is. I'll start listing some of them, but please keep in mind C++ is a rather complex language, and there's a number of things that go on behind the scenes compared to C. Depending on which C++ version you are using (there are multiple standards, and certain compiles support things differently), there's a lot of stuff to learn and become familiar with if you venture into this side of things.

Your new/delete operators are not paired correctly:

this->__data = new T[capacity];

delete this->__data; //program crashes here, but the pointer is already broken...

new[] has to be paired with delete[], and new has to be paired with delete. Mixing them up results in undefined behavior (which if you're lucky, will result in crashes, and if you're unlucky, results in things still working).

The next issue is really obscure, but you need to understand the difference of passing by reference and passing by value. Basically, this is why you should always post all your code when asking for help with this stuff, because your print_vec most likely also contains a bug.

Say for example your print_vec looked like this:

template <typename T>
void print_vec(vector<T> vec)
{
	for(auto x = 0; x < vec.size(); ++x)
	{
		std::cout << vec[x] << std::endl; // C++, but you can use C's printf
	}
}

Can you spot the bug? Unless you're experienced in C/C++, you won't because the bug is an unintended side effect of something that happens behind the scenes when you pass by value.

When you pass by value, a copy of the object is created on the stack that the function will use. This is fine for simple data types, but since you are using C++, you are passing a class object, which results in a copy constructor being called, and a destructor being called when the object's scope is left.

To illustrate this, change your destructor to the following:

~vector() {

	std::cout << "~vector" << std::endl; // or use printf

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

	this->__count = this->__capacity = 0;
}

If you run your program in Visual Studio (not familiar with Arduino's io functions) you'll see your destructor is called after the first print_vec call.

When your next push_back is called, the memory was just deleted in the destructor, so you get a crash!

One "fix" for this is to make your print_vec function pass by reference instead, and make some changes to your public functions to be const (non-modifying) for anything that simply "gets".

template <typename T>
void print_vec(const vector<T> & vec)
{
	for(auto x = 0; x < vec.size(); ++x)
	{
		std::cout << vec[x] << std::endl;
	}
}

...

int size() const {
	return this->__count;
}

int capacity() const {
	return this->__capacity;
}

A similar issue also exists with your push_back function. Since it passes by value as well, you're going to copy objects which might lead to undesirable behavior depending on if class objects are used.

However, this leads to more C++ class design issues with your current code. Basically, the code doesn't conform to C++ class design correctly, so there's a lot of work you need to do for this to work correctly.

You need to implement a proper copy constructor for your class. Look up the "rule of 3 c++" on Google for example.

Next, if you accept C++ objects via the template, your memcpy will not result in intended object copying, since you're performing a raw memory copy rather than going though C++'s class copy/assignment operators (which can vary by class implementation).

In your constructor, the initial data check is incorrect because when your object is allocated on the stack (or via placement new), previous memory is not guaranteed to be zeroed out, so you will be deleting random or invalid memory, which will result in really hard to track down issues.

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

Basically, you should never be using 'this' memory before you initialize it, so you can remove that check there.

There's a few other things, but basically writing proper C++ code is tricky, and you have to be aware of a lot of little things. I'd suggest trying to find a code base that does all this for you and seeing if you can adopt the implementation, or resort to a simpler, C-based design where you have more control over things (at the trade-off of more code that doesn't look as pretty).

In any case, good luck!

[Hitokiri~]

Also one thing you may want to consider to better conform to the C++ standards would be to use a smart pointer to avoid having to worry about deallocation.