471,071 Members | 1,405 Online
Bytes | Software Development & Data Engineering Community
Post +

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 471,071 software developers and data experts.

class with destructor inside a vector...?

Hello all,
I have encountered with following strange problem.
I am coding in C++ and using VC++ 6 compiler.

I have a class strvector containing char * cstr as a private member
and i have defined its destructor for releasing memory hold by cstr.

In main i created a vector<strvector>
and whenever i put a object of strvector inside this vector the program
crashes.

The complete program is as follows
/////////////////////////////////////////
#include <iostream.h>
#include "string"
#include "vector"

using namespace std;

class strvector
{
char * cstr;

public:

char * getcstr()
{
return cstr;
}
strvector(char * s)
{
cstr = new char[strlen(s)+1];
strcpy(cstr,s);
}
~strvector()
{
delete str;
}
/*release()
{
delete cstr;
}*/
};

void main()
{
vector<strvector> vec;

strvector str("one");
vec.push_back(str);//The program will crash
//str.release();

//cout<<vec[0].getcstr();//will print a garbage values.
}
///////////////////////////////////
If i remove the destructor and define a separate member function
release for releasing the memory,the cout statement will print the
garbase value.

Will someone please tell me what is going wrong in the above program
even if i am following a good programmering approach.(using destructor
etc.)?

Regards,
Yogesh Joshi

Sep 28 '05 #1
12 1720
yp*********@indiatimes.com wrote:
strvector(char * s)
{
cstr = new char[strlen(s)+1];
strcpy(cstr,s);
}
~strvector()
{
delete str;
If you new[], you need to delete[].
}

Also, read about "the Rule of Three".

V
Sep 28 '05 #2
>If you new[], you need to delete[].
Still its crashing..Also i tried to put the copy constructor but no
improvement..still crashing..The code will run fine only when i remove
the destructor and will not release the memory held by cstr..

regards,
Yogesh

Sep 28 '05 #3
yp*********@indiatimes.com wrote:
If you new[], you need to delete[].


Still its crashing..Also i tried to put the copy constructor but no
improvement..still crashing..The code will run fine only when i remove
the destructor and will not release the memory held by cstr..

regards,
Yogesh


The rule of three states that you need a destructor, copy constructor
and assignment operator.

If it is still crashing then post all the code. The bug is almost
certainly in the code for your copy constructor or assignment operator.

john
Sep 28 '05 #4
John, as per your suggestion i tried to put copy constructor,
assignment operator and destructor..the problem got worst..it can't
even compile..
below is the code
////////////////////////////////// code starts
////////////////////////////////
#include <iostream.h>
#include "string"
#include "vector"

using namespace std;

class strvector
{
char * cstr;

public:
strvector()
{
}

strvector(strvector & s)
{
cstr = new char[strlen(s.cstr) + 1];
strcpy(cstr,s.cstr);
}
strvector(char * s)
{
cstr = new char[strlen(s)+1];
strcpy(cstr,s);
}

strvector & operator =(strvector & s)
{
cstr = new char[strlen(s.cstr) + 1];
strcpy(cstr,s.cstr);
return *this;

}

char * getcstr()
{
return cstr;
}

~strvector()
{
delete []cstr;
}
/* release()
{
delete cstr;
}
*/
};

void main()
{
vector<strvector> vec;

strvector str("one");
vec.push_back(str);
// str.release();

//cout<<vec[0].getcstr();
}
////////////////////////////////// code ends
//////////////////////////////////
and below are the compile errors

--------------------Configuration: Cpp2 - Win32
Debug--------------------
Compiling...
Cpp2.cpp
c:\program files\microsoft visual studio\vc98\include\xutility(39) :
error C2679: binary '=' : no operator defined which takes a right-hand
operand of type 'const class strvector' (or there is no acceptable
conversion)
c:\program files\microsoft visual studio\vc98\include\vector(170) : see
reference to function template instantiation 'void __cdecl
std::fill(class strvector *,class strvector *,const class strvector &)'
being compiled
c:\program files\microsoft visual studio\vc98\include\xmemory(34) :
error C2558: class 'strvector' : no copy constructor available
c:\program files\microsoft visual studio\vc98\include\xmemory(66) : see
reference to function template instantiation 'void __cdecl
std::_Construct(class strvector *,const class strvector &)' being
compiled
Error executing cl.exe.

Cpp2.obj - 2 error(s), 0 warning(s)
regards,
Yogesh Joshi

Sep 28 '05 #5
yp*********@indiatimes.com wrote:
John, as per your suggestion i tried to put copy constructor,
assignment operator and destructor..the problem got worst..it can't
even compile..
You need to get familiar with the concept of "const-correctness". See
my notes inline with the code.
below is the code
////////////////////////////////// code starts
////////////////////////////////
#include <iostream.h>
#include "string"
#include "vector"

using namespace std;

class strvector
{
char * cstr;

public:
strvector()
{
}

strvector(strvector & s)
strvector(strvector const & s)
{
cstr = new char[strlen(s.cstr) + 1];
strcpy(cstr,s.cstr);
}
strvector(char * s)
strvector(char const * s)
{
cstr = new char[strlen(s)+1];
strcpy(cstr,s);
}

strvector & operator =(strvector & s)
strvector & operator =(strvector const & s)
{
cstr = new char[strlen(s.cstr) + 1];
strcpy(cstr,s.cstr);
return *this;

}

char * getcstr()
{
return cstr;
}

~strvector()
{
delete []cstr;
}
/* release()
{
delete cstr;
}
*/
};

void main()
int main
{
vector<strvector> vec;

strvector str("one");
vec.push_back(str);
// str.release();

//cout<<vec[0].getcstr();
}
////////////////////////////////// code ends
[..]


And spend some time reading the FAQ, it will help tremendously.

V
Sep 28 '05 #6
yp*********@indiatimes.com wrote:
John, as per your suggestion i tried to put copy constructor,
assignment operator and destructor..the problem got worst..it can't
even compile..
below is the code
////////////////////////////////// code starts
////////////////////////////////
#include <iostream.h>
#include "string"
#include "vector"
Should be

#include <iostream>
#include <string.h>
#include <vector>

using namespace std;

class strvector
{
char * cstr;

public:
strvector()
{
}
This is wrong because cstr will be garbage and you will end up deleteing
a garbage pointer. At least you should do

strvector()
{
cstr = 0;
}

strvector(strvector & s)
Should be,

strvector(const strvector & s)
{
cstr = new char[strlen(s.cstr) + 1];
strcpy(cstr,s.cstr);
}
strvector(char * s)
{
cstr = new char[strlen(s)+1];
strcpy(cstr,s);
}

strvector & operator =(strvector & s)
Should be

strvector & operator =(const strvector & s)

You cannot get by in C++ without understanding const.
{
cstr = new char[strlen(s.cstr) + 1];
strcpy(cstr,s.cstr);
return *this;

This leaks memory. Try this

if (&s != this)
{
delete[] cstr;
cstr = new char[strlen(s.cstr) + 1];
strcpy(cstr,s.cstr);
}
return *this;

}

char * getcstr()
{
return cstr;
}

~strvector()
{
delete []cstr;
}
/* release()
{
delete cstr;
}
*/
};

void main()
{
vector<strvector> vec;

strvector str("one");
vec.push_back(str);
// str.release();

//cout<<vec[0].getcstr();
}
////////////////////////////////// code ends
//////////////////////////////////


Now for me it compiles and runs without errors.

john
Sep 28 '05 #7
yp*********@indiatimes.com wrote in news:1127943189.045541.187280
@o13g2000cwo.googlegroups.com:
John, as per your suggestion i tried to put copy constructor,
assignment operator and destructor..the problem got worst..it can't
even compile..
below is the code
Comments inline:
////////////////////////////////// code starts
////////////////////////////////
// Ahem... <iostream.h> is non-Standard
// use <iostream> #include <iostream.h>
#include "string"
#include "vector"

using namespace std;

class strvector
{
char * cstr;

public:
strvector() : cstr(0) // Initialize the pointer to 0 {
}

strvector(strvector & s) // This isn't a Copy Constructor (or at least
// not a normal one.... you should have:
strvector(const strvector & s) {
cstr = new char[strlen(s.cstr) + 1];
strcpy(cstr,s.cstr);
}
strvector(char * s)
{
cstr = new char[strlen(s)+1];
strcpy(cstr,s);
}

strvector & operator =(strvector & s)
{
cstr = new char[strlen(s.cstr) + 1];
strcpy(cstr,s.cstr);
return *this;

}

char * getcstr()
{
return cstr;
}

~strvector()
{
delete []cstr;
}
/* release()
{
delete cstr;
}
*/
};

void main()
{
vector<strvector> vec;

strvector str("one");
vec.push_back(str);
// str.release();

//cout<<vec[0].getcstr();
}
////////////////////////////////// code ends
//////////////////////////////////
and below are the compile errors

--------------------Configuration: Cpp2 - Win32
Debug--------------------
Compiling...
Cpp2.cpp
c:\program files\microsoft visual studio\vc98\include\xutility(39) :
error C2679: binary '=' : no operator defined which takes a right-hand
operand of type 'const class strvector' (or there is no acceptable
conversion)
c:\program files\microsoft visual studio\vc98\include\vector(170) : see
reference to function template instantiation 'void __cdecl
std::fill(class strvector *,class strvector *,const class strvector &)'
being compiled
c:\program files\microsoft visual studio\vc98\include\xmemory(34) :
error C2558: class 'strvector' : no copy constructor available
As above.. you don't have a proper copy constructor...
c:\program files\microsoft visual studio\vc98\include\xmemory(66) : see
reference to function template instantiation 'void __cdecl
std::_Construct(class strvector *,const class strvector &)' being
compiled
Error executing cl.exe.

Sep 28 '05 #8
yes..i could compile the code and ran it without fail..
Thanks Victor,John and Adre!!!

Thanks and Regards,
Yogesh Joshi

Sep 28 '05 #9
Just out of curiosity. I tried to run the original code submitted by
Yogesh on Linux platform and it seems to work fine. I was wondering if
somebody could throw some light on that issue.

Sep 29 '05 #10
Just out of curiosity. I tried to run the original code submitted by
Yogesh on Linux platform and it seems to work fine. I was wondering if
somebody could throw some light on that issue.

Sep 29 '05 #11
kamit wrote:
Just out of curiosity. I tried to run the original code submitted by
Yogesh on Linux platform and it seems to work fine. I was wondering if
somebody could throw some light on that issue.


Yogesh's original code freed the same memory twice. The pointer str.cstr
would get freed twice, once when the vector was destroyed and once when
variable str was destroyed. This is because without a copy constructor
the str.cstr pointer would just be copied unchanged into the vector.
Adding a proper copy constructor fixed this problem.

But freeing the same memory twice does not necessarily mean that your
program will crash. It is an exampe of what C++ calls 'undefined
behaviour'. This means that C++ doesn't know how to handle the situation
and *anything* could happen. Yogesh saw a crash on his system, but you
didn't. That's OK, undefined behaviour means anything could happen.
Obviously if you want your programs to work you don't want undefined
behaviour, but just because you do have undefined behaviour doesn't mean
that your programs won't work. This is one thing that makes debugging
C++ tricky.

john
Sep 29 '05 #12
thanks for the feedback john. I really appreciate it.

Sep 29 '05 #13

This discussion thread is closed

Replies have been disabled for this discussion.

Similar topics

5 posts views Thread by Bruce Lee Roy | last post: by
3 posts views Thread by alexhong2001 | last post: by
3 posts views Thread by Hamilton Woods | last post: by
2 posts views Thread by maynard | last post: by
7 posts views Thread by Thomas | last post: by
1 post views Thread by Rune Allnor | last post: by
reply views Thread by leo001 | last post: by

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.