Access violation reading location

[Spot]

Hi everyone!

My code builds fine but I'm getting the error: Unhandled exception at 0x00413b62 in Tenta_1_del_3.exe: 0xC0000005: Access violation reading location 0xfeeefeee when I'm running it in debug-mode.

My constructors, destructors, copyconstructor and overloaded assignment operator should be correct so I have no idea why this happens.

I would very much appreciate if someone could take a look at my code below cause my teacher couldn't help me. He had no idea!

There's something about a bad pointer, that's all I've figured out.

(There's two more files I haven't uploaded here (MoneyBox.h and .cpp) but they should be fine. The problem must be in MoneyBoxHandler)

Thanks.

#include <iostream>
#include "MoneyBoxHandler.h"

MoneyBoxHandler::MoneyBoxHandler()
{
	this->capacity=3;
	this->nrOfBoxes=0;

	this->moneyBoxes=new MoneyBox*[this->capacity];

	for(int i=0;i<this->capacity;i++)
		this->moneyBoxes[i]=NULL;
}

MoneyBoxHandler::MoneyBoxHandler(int size)
{
	this->capacity=size;
	this->nrOfBoxes=0;

	this->moneyBoxes=new MoneyBox*[this->capacity];

	for(int i=0;i<this->capacity;i++)
		this->moneyBoxes[i]=NULL;
}

MoneyBoxHandler::~MoneyBoxHandler()
{
	for(int i=0;i<this->nrOfBoxes;i++)
		delete this->moneyBoxes[i];

	delete[]this->moneyBoxes;
	this->moneyBoxes=NULL;
}

MoneyBoxHandler::MoneyBoxHandler(const MoneyBoxHandler &obj)
{
	this->capacity=obj.capacity;
	this->nrOfBoxes=obj.nrOfBoxes;

	this->moneyBoxes=new MoneyBox*[this->capacity];

	for(int i=0;i<this->capacity;i++)
		this->moneyBoxes[i]=NULL;

	for(int i=0;i<obj.nrOfBoxes;i++)
		this->moneyBoxes[i]=obj.moneyBoxes[i];
}

MoneyBoxHandler& MoneyBoxHandler::operator=(const MoneyBoxHandler &obj)
{
	this->capacity=obj.capacity;
	this->nrOfBoxes=obj.nrOfBoxes;
	
	this->moneyBoxes=new MoneyBox*[this->capacity];

	for(int i=0;i<this->capacity;i++)
		this->moneyBoxes[i]=NULL;

	for(int i=0;i<obj.nrOfBoxes;i++)
		this->moneyBoxes[i]=obj.moneyBoxes[i];

	return *this;
}

void MoneyBoxHandler::addMoneyBox(string owner)
{
	if(this->nrOfBoxes==this->capacity)
	{
		this->capacity+=2;
		MoneyBox **temp=new MoneyBox*[this->capacity];

		for(int i=0;i<this->capacity;i++)
			temp[i]=NULL;

		for(int i=0;i<this->nrOfBoxes;i++)
			temp[i]=this->moneyBoxes[i];

		delete[]this->moneyBoxes;
		this->moneyBoxes=temp;
		temp=NULL;
	}

	this->moneyBoxes[this->nrOfBoxes++]=new MoneyBox(owner,0);
}

void MoneyBoxHandler::addMoneyToMoneyBox(string owner, int amount)
{
	bool done=false;

	for(int i=0;i<this->nrOfBoxes&&!done;i++)
	{
		if(this->moneyBoxes[i]->getOwner()==owner)
		{
			this->moneyBoxes[i]->add(amount);
			done=true;
		}
	}
}

void MoneyBoxHandler::showAll()const
{
	for(int i=0;i<this->nrOfBoxes;i++)
	{
		cout<<this->moneyBoxes[i]->getOwner()<<endl;
		cout<<this->moneyBoxes[i]->getBalance()<<endl;
	}
}

#include "MoneyBoxHandler.h"
#include <iostream>

void test1(MoneyBoxHandler &mbh)
{
	mbh.addMoneyToMoneyBox("Pelle", 200);
}

void test2(MoneyBoxHandler &mbh)
{
	mbh.addMoneyToMoneyBox("Stina", 300);
	mbh.addMoneyToMoneyBox("Stina", 100);
}

MoneyBoxHandler test3()
{
	MoneyBoxHandler mbh(3);
	mbh.addMoneyBox("Kajsa");
	mbh.addMoneyToMoneyBox("Kajsa", 800);
	return mbh;
}

void print(MoneyBoxHandler &mbh)
{
	mbh.showAll();
}

int main()
{
	_CrtSetDbgFlag( _CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
	MoneyBoxHandler mbh1(6);
	MoneyBoxHandler mbh2(8);
	
	mbh1.addMoneyBox("Kalle");
	mbh1.addMoneyBox("Pelle");
	mbh2.addMoneyBox("Stina");

	test1(mbh1);
	test2(mbh2);

	MoneyBoxHandler mbh3 = mbh1;
	MoneyBoxHandler mbh4;

	mbh4 = mbh2;
	
	print(mbh1);
	print(mbh2);
	print(mbh3);
	print(mbh4);
	
	mbh4 = test3();
	print(mbh4);

	MoneyBoxHandler mbh5 = MoneyBoxHandler(mbh3);

	return 0;
}

[hanst99]

In your copy constructor:

	for(int i=0;i<obj.nrOfBoxes;i++)
		this->moneyBoxes[i]=obj.moneyBoxes[i];

should be

this->moneyBoxes[i] = new MoneyBox(*(obj.moneyBoxes[i]);

Otherwise you will just be copying the pointers--- this is probably not what you want, else you will delete your money boxes twice when your handlers go out of scope. This is probably what causes your error, fix that and tell me if something is still wrong.

Oh btw: In your assignment operator, delete your moneyboxes instead of just setting them to NULL, else you will leak memory here.

[Spot]

Thank you hanst99.

I did the changes but I'm still getting the error. This is so frustrating. Any other ideas?

[Spot]

No, it's working now. My bad. Problem solved! Thanks!

[Disch]

also, fwiw...

putting this-> before all your member vars is redundant an unnecessary. You can omit that to save yourself some typing.

[Spot]

I really didn't know that. What's the use of this-> anyways? When do you HAVE to use it?

[firedraco]

It's there to clarify if you have another variable that has the same name.

[Zhuge]

e.g.

class MyClass {
    int var;
    void func(int var) {
        var = 2; // modifies parameter
        this->var = 2; // modifies class member
    }
};

[Disch]

It's also worth nothing that the above cases can be avoided if you are more careful with naming your variables.