Freeing memory exception

[gabriela]

Hello, i am trying to make a vector class for exercise purpose but i am having trouble with memory management.

This is my Vector.cpp

 #include "Vector.h"
#include <iostream>
const int INITIAL_CAPACITY = 4;
Vector::Vector() :size(0), capacity(INITIAL_CAPACITY), data(nullptr), name(nullptr)
{
}

Vector::Vector(const Vector& rhs)
{
	copy(rhs);
}

void Vector::copy(const Vector& rhs){

	setSize(rhs.getSize());
	setCapacity(rhs.getCapacity());
	setName(rhs.getName());
	setData(rhs.getData(), this->size);
}

char* Vector::getName()const{
	if (name){
		int len = strlen(this->name);
		char* copy = new char[len + 1];
		strcpy_s(copy, len + 1, name);
		return copy;
	}
}
double* Vector::getData()const{
	if (data){
		double* copy = new double[size];
		for (int i = 0; i < size; i++)
			copy[i] = data[i];
		return copy;
	}
}
void Vector::setName(const char* name){
	int len = strlen(name);
	delete[] this->name;
	this->name = new(std::nothrow) char[len + 1];
	if (name)
		strcpy_s(this->name, len + 1, name);
}

Vector& Vector::operator=(const Vector& rhs){
	if (this != &rhs)
	{
		copy(rhs);
	}
	return *this;
}

void Vector::setSize(const int size){
	this->size = size;
}
void Vector::setCapacity(const int capacity){
	this->capacity = capacity;
}
void Vector::setData(const double* data, const int size){
	if (data){
		delete[] this->data;
		double* arr = new double[size];
		for (int i = 0; i < size; i++)
			arr[i] = data[i];
		this->data = arr;
	}

}
int Vector::getSize()const{
	return size;
}

int Vector::getCapacity()const{
	return this->capacity;
}


void Vector::add(const double value){
	if (!data)
		data = new(std::nothrow) double[capacity];
	if (capacity == size){
		resize(size * 2);
	}
	data[size++] = value;
}

void Vector::resize(const int size){
	capacity = size;
	double* arr = new double[capacity];
	for (int i = 0; i < this->size; i++)
		arr[i] = data[i];
	delete[] data;
	data = arr;
}

void Vector::del(const int index){
	if (index >= 0 && index < size){
		double* copy = new double[size - 1];
		int j = 0;
		for (int i = 0; i < size; i++){
			if (i == index)
				i++;
			copy[j] = data[i];
			data[i] = 0;
			data[j] = copy[j];
			j++;
		}
	}
	size--;
}


double Vector::operator[] (const int index)const{

	if (index > 0 && index < size){
		return data[index];
	}
}
double& Vector::operator[] (const int index)
{

	if (index > 0 && index < size)
		return data[index];

}
Vector::~Vector(){
	delete[] name;
	delete[] data;
}

std::ostream& operator<<(std::ostream& out, const Vector& v){
	out << "name: " << v.name << std::endl << "data: ";
	for (int i = 0; i < v.size; i++){
		out << i << ": " << v.data[i] << ", ";
	}
	return out;
}

std::istream& operator>>(std::istream& in, Vector& v){
	int size;
	in >> size;
	if (size > 0){
		v.size = size;
		v.capacity = size * 2;
		delete[] v.data;
		v.data = new double[size];
	}
	for (int i = 0; i < size; i++)
		in >> v.data[i];
	return in;
}

This is Vector.h

#ifndef _VECTOR_H_
#define _VECTOR_H_

#include <fstream>
class Vector{
public:
	Vector();
	Vector(const Vector& rhs);
	Vector& operator=(const Vector& rhs);
	~Vector();

	char* getName()const;
	void setName(const char* name);
	int getSize()const;
	int getCapacity()const;
	double* getData()const;
	void add(const double value);
	void resize(const int size);
	void del(const int index);
	void copy(const Vector& rhs);

	double operator[] (const int index)const;
	double& operator[] (const int index);


	friend std::ostream& operator<<(std::ostream& out, const Vector& v);
	friend std::istream& operator>>(std::istream& in, Vector& v);
private:
	void setSize(const int size);
	void setCapacity(const int capacity);
	void setData(const double* data, const int size);

private:
	int size;
	int capacity;
	double* data;
	char* name;

};

And this is my main:

#include <iostream>
#include <fstream>
#include <string>
#include "Vector.h"


using namespace std;


int main()
{
	Vector v;
	v.setName("gabi");
	v.add(4);
	v.add(2);
	v.add(2.2);
	v.add(5);
	v.add(6);
	cout << v;
	Vector v2 = v;
	v2.add(78);
	v.add(78);

	v2.setName("alabala");
	cout << v2;
	system("pause");
	return 0;
}

I only found that the exception comes from destructor when i call delete[] data, but dont know what is the reason for it. Please if someone knows where is my mistake, let me know!

[Thomas1965]

Actually your program crashes in the function setName called from copy constructor when you assign v to v2 on line 20 in main.

[Cubbi]

i am trying to make a vector class for exercise purpose but i am having trouble with memory management.

Keep in mind that real C++ code would never do any of this: your name would be std::string, your data would be std::vector<double>, and you would not define a copy constructor, an assignment operator, or a destructor. You're not exercising a practical skill. Of course if you are really comfortable writing real C++, it may be worth exploring this dusty corner.

That said, the biggest errors are:

1. Vector::setName() calls delete[] this->name;, but when it is called from Vector's copy constructor, there's garbage in this->name

2. Vector::getName and Vector::getData have no return statements when the condition is false

3. Vector::setData uses size instead of capacity in double* arr = new double[size];. The first call to v2.add will write to unallocated memory with data[size++] = value;

4. memory leaks: the pointers returned by getName and getData are never freed.

It's worth rewriting this using at least the 1998 C++, or, better, modern C++.