Where should delete my pointer in this program?

[Alkhzar]

Hello all! I've been working with a book, learning about overloading operators. I went a veered off the path and went ahead and made "itsAge" into a pointer and dynamically allocated memory for the pointer in the class constructor. The thing is, when I try to delete the pointer in the destructor, the program crashes. If I can't delete the pointer in the destructor, then where do I delete it?

Here's the code:

#include <iostream>

typedef unsigned short USHORT;

class Counter
{
    public:
        Counter();
	Counter(USHORT);
	Counter(const Counter &);
	~Counter() {};
	USHORT getItsValue() const { return *itsVal; }
	void setItsValue(USHORT val) { *itsVal = val; }
	void increment() { ++*itsVal; }
	const Counter operator++();
	const Counter operator++(int);
	const Counter operator--();
	const Counter operator--(int);
	Counter add(const Counter &);
	Counter operator+(const Counter &);
    private:
	USHORT *itsVal;
};

Counter::Counter()
{
    itsVal = new USHORT;
    *itsVal = 0;
}

Counter::Counter(USHORT value)
{
    itsVal = new USHORT;
    *itsVal = value;
}

Counter::Counter(const Counter &rhs)
{
    itsVal = new USHORT;
    *itsVal = rhs.getItsValue();
}

const Counter Counter::operator++()
{
    ++*itsVal;
    return *this;
}

const Counter Counter::operator++(int)
{
    Counter temp(*this);
    ++*itsVal;
    return temp;
}

const Counter Counter::operator--()
{
    --*itsVal;
    return *this;
}

const Counter Counter::operator--(int)
{
    Counter temp(*this);
    --*itsVal;
    return temp;
}

Counter Counter::add(const Counter & rhs)
{
    return *this->itsVal + rhs.getItsValue();
}

Counter Counter::operator+(const Counter & rhs)
{
    return *this->itsVal + rhs.getItsValue();
}

int main()
{
	
    Counter i, j(i);
    Counter one(2), two(4), three;
    std::cout << "The value of i is " << i.getItsValue() << "\n";
    i.increment();
    std::cout << "The value of i is " << i.getItsValue() << "\n";
    ++i;
    std::cout << "The value of i is " << i.getItsValue() << "\n";
    Counter a(++i);
    std::cout << "The value of a is " << a.getItsValue();
    std::cout << " and i is " << i.getItsValue() << "\n";
    a = i++;
    std::cout << "The value of a is " << a.getItsValue();
    std::cout << " and i is " << i.getItsValue() << "\n";
    --a;
    --i;
    i = a--;
    i--;
    std::cout << "The value of a is " << a.getItsValue();
    std::cout << " and i is " << i.getItsValue() << "\n\n\n";
    three = one.add(two);
    std::cout << "Three's value is: " << three.getItsValue() << "\n";
    ++one;
    ++two;
    three = one + two;
    std::cout << "Three's value is: " << three.getItsValue() << "\n";
    std::cin.sync();
    std::cin.get();
    return 0;
}

I was thinking of making a public delete function that deletes the pointer, but I don't know if that is very safe or not.

Here is the function i was referring to:

void deleteItsVal() { delete itsVal; itsVal = 0; }

Then calling it in main:

i.deleteItsVal();

If there is a way I can use delete in my destructor, I would like to know how to do that.

Any help would be appreciated!

[Athar]

No, the delete belongs into the destructor. See:
http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization

As for the problem, you forgot to overload operator=. For reference:
http://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29

[JellyFox]

You can (and should) delete itsVal in your destructor. Your code is failing because you've forgotten to write an assignment operator. Add something like this:

const Counter& Counter::operator=(const Counter& rhs)
{
	*itsVal = rhs.getItsValue();
	return *this;
}

Normally you would check for self-assignment, but since you don't have to re-allocate the memory, this will work. Otherwise, when you do something like this:

a = i++;

What you're really doing is using the compiler's synthesised assignment operator on the temporary object that the postfix ++ operator returns. The synthesized assignment operator copies the pointer, but the object is destroyed as soon as the assignment is completed. Its destructor is called, freeing the memory, but a is left pointing to the memory that it just freed. What's more, the old pointer was replaced, but the memory it pointed to wasn't freed, so you have a memory leak too.

[Alkhzar]

Wow! Thanks, guys!

A note on this part:

a = i++;

^ I obviously knew that I was using the assignment operator but I didn't know how I could use it without overloading it. Thanks for clearing that up!

What you're really doing is using the compiler's synthesised assignment operator on the temporary object that the postfix ++ operator returns.

My book failed to mention the compiler's synthesized assignment operator, but I assumed there was something happening that I hadn't learned yet.

Maybe I just tried to do more than I should be doing with my current knowledge of c++... Haha, this happens a lot to me. I should just stick to copying what the author wrote for now, lol.

Its destructor is called, freeing the memory, but a is left pointing to the memory that it just freed. What's more, the old pointer was replaced, but the memory it pointed to wasn't freed, so you have a memory leak too.

^ Would overloading the assignment operator and using it correctly fix all those errors?

[Athar]

Would overloading the assignment operator and using it correctly fix all those errors?

Yes.