0

For some reason, when I use the destructor for my Update class, a debug assertion fail message displays...

here is my Update class with some code omitted for brevity. Placed in a header file:

using namespace std;

class Update
{
private:
    int day, month, year;
static const int FIELD_SIZE = 3, DEFAULT_DAY = 12, DEFAULT_MONTH = 12,
    DEFAULT_YEAR = 1999, DAYS_IN_MONTH = 30, MONTHS_IN_YEAR = 12, DAYS_IN_YEAR = 365;

int * date;

public:
static int dateUpdate;

Update(int D, int M, int Y)
{
    day = D;
    if (day < 1 || day > DAYS_IN_MONTH)
        day = DEFAULT_DAY;
    month = M;
    if (month < 1 || month > MONTHS_IN_YEAR)
        month = DEFAULT_MONTH;
    year = Y;
    if (year < 1)
        year = DEFAULT_YEAR;

    date = new int [FIELD_SIZE];
    date[0] = day, date[1] = month, date[2] = year;

    dateUpdate++;
}

~Update()
{
    delete [] date;

    dateUpdate--;
}

};

and here is my tester class in a cpp file:

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

int Update::dateUpdate = 0;

int main()
{
Update u1(29, 12, 2000);
u1.Update::~Update();

return 0;
}

I've read through other questions involving debug assertion failures but something tells me a debug assertion failure can occur in various ways. As a result, I have little clue as to why the error message is displaying for my code... Is there something wrong with my destructor as I suspect at the moment? Thank you so much for your help in advance!

user1800967
  • 953
  • 1
  • 6
  • 11
  • 3
    Why do you call the destructor explicitly? – olevegard May 05 '13 at 19:24
  • Why do you use `new[]` and `delete[]` instead of a container that does the heavy lifting for you (which you've done incorrectly as far as the example goes)? – chris May 05 '13 at 19:25
  • Is that not how it's done? I was looking at msdn.microsoft.com/en-us/library/35xa3368%28v=vs.80%29.aspx at the time... – user1800967 May 05 '13 at 19:25
  • @chris well it was to create an array in dynamic memory – user1800967 May 05 '13 at 19:26
  • @user1800967, That page makes it very clear when to use it, and that it's seldom necessary. For a dynamic array, just use `std::vector` or likewise. – chris May 05 '13 at 19:27
  • @user1800967 I think you are doing it correctly... But why? The destructor will be called at the end of the scope anyways. – olevegard May 05 '13 at 19:27
  • @olevegard Oh. I see... So then there's no need for a call to the destructor... C++ calls it on its own... Now that you mention it, I recall reading something to that effect... – user1800967 May 05 '13 at 19:29

2 Answers2

2

The problem is because you are calling destructor explicitly:

u1.Update::~Update();

this way it is called twice causing undefined behaviour, I suppose delete [] date; is called twice, the second time on alreade freed memory.

Another problem in your code is that you are using bare pointer for you array:

int * date;

this is actually quite low level programming style in C++ and can cause lots of problems. You should implement class copy constructor and assignment operator (*) that will allocate new date array when your Update class will be copied, otherwise you will have again problems with multiple date pointer deletions.

The best way is to use vector like

std::vector<int> date;

(*) I think good link where rule of three (or since C++11 rule of five) that applies here is explained: Rule-of-Three becomes Rule-of-Five with C++11?

Community
  • 1
  • 1
marcinj
  • 48,511
  • 9
  • 79
  • 100
  • And this is why manual memory management is so bad. You neglected to mention the need for an assignment operator, and move versions of these would be required in C++11. Also, calling the destructor on an object twice is, in itself, undefined behaviour, even if it's empty. – chris May 05 '13 at 19:42
1

You should change this line:
date[0] = day, date[1] = month, date[2] = year;

To:

date[0] = day;
date[1] = month;
date[2] = year;

You are using the comma operator, which returns the result of the last expression. This is not the same as how the comma operates in an initialization.

Also, in your main function, you do not need to explicitly call the destructor.

Your date method can't handle January 31 nor December 31.

You can access the parameters of the function directly instead of making a copy in the function. For example: day = D; is not needed; access the parameter directly: if ((D < 1) ....

If you used unsigned integers you would not need to check for negative numbers. I've never experienced a negative day, month or year in my life.

Since this is C++ and not Java or C#, you don't need to dynamically allocate variables. So rather than using int * date you could use int date[3].

Thomas Matthews
  • 56,849
  • 17
  • 98
  • 154
  • That use of the comma is still valid (it will assign each value to each variable), but definitely not good practice. – chris May 05 '13 at 19:30
  • For the specific assignment I'm working on, negative numbers are fine. So it's cool. Thanks. – user1800967 May 05 '13 at 19:47