Problem with constructor call of a string class

[dragoncoder]

Hello experts,

I have the following code me.

=> cat mystring.h
#include<iostream>
using namespace std;

class mystring
{
char* p;
int capacity;
int len;
enum {DEF_SIZE = 100};
public:
// Constructors
mystring();
mystring(int size);
mystring(const char* str);

// Iniializer
void init(int size = DEF_SIZE);

//Destructor
~mystring();

//Copy Constructor
mystring(mystring&);

//Assignment Operator
mystring& operator = (mystring&);

//Display String
void display();

};

=> cat mystring.cpp
#include<iostream>
using namespace std;

#include "mystring.h"

//Constructors
mystring::mystring()
{
cout << "Default Constructor" << endl;
init();
len = 0;
}

mystring::mystring(int size)
{
cout << "Constructor having size" << endl;
init(size);
len = 0;
}

mystring::mystring(const char* str)
{
cout << "Constructor with string argument" << endl;
int length = strlen(str);
init(length + 1);
len = length + 1;
strcpy(p, str);
}

void mystring::init(int size)
{
p = new char[size];
capacity = size;
}

mystring::~mystring()
{
cout << "Destructor" << endl;
delete p;
p = NULL;
}
mystring::mystring(mystring& other)
{
cout << "Copy Constructor" << endl;
delete p;
p = new char[strlen(other.p) + 1];
strcpy(p,other.p);
}

mystring& mystring:perator=(mystring& other)
{
cout << "Assignment Operator" << endl;
if(this == &other)
return *this;
delete p;
p = new char[strlen(other.p)+1];
strcpy(p,other.p);
return *this;
}

void mystring::display()
{
cout << *p << endl;
cout << "Length of string is " << len << endl;
cout << "Capacity of the string is " << capacity << endl;
}

=> cat str.cpp
#include<iostream>
using namespace std;
#include "mystring.h"

int main()
{
mystring s = "Nitin";
return 0;
}

Compiling this gives error.

=> g++ mystring.cpp str.cpp
str.cpp: In function `int main()':
str.cpp:7: no matching function for call to
`mystring::mystring(mystring)'
mystring.h:24: candidates are: mystring::mystring(mystring&)
mystring.h:15: mystring::mystring(const char*)
mystring.h:14: mystring::mystring(int)
str.cpp:7: initializing temporary from result of
`mystring::mystring(const
char*)'

Changing the parameter of the copy constructor to "const mystring&"
solves the problem. Can someone please explain how it makes a
difference ?

Thanks in advance.

 

[Alf P. Steinbach]

* dragoncoder:

Changing the parameter of the copy constructor to "const mystring&"
solves the problem. Can someone please explain how it makes a
difference ?

A temporary (the actual argument) can't be bound to a reference to
non-const.

 

[mike]

the form for a copy constructor uses the const key word. When you use
the = operator, it calls the copy constructor. Since you don't have a
mystring(const mystring&) constructor, the compiler doesn't really
know what your copy constructor is.

 

[Alf P. Steinbach]

* mike:

the form for a copy constructor uses the const key word. When you use
the = operator, it calls the copy constructor. Since you don't have a
mystring(const mystring&) constructor, the compiler doesn't really
know what your copy constructor is.

First, please don't top-post in this newsgroup -- see the FAQ.

Now, there are an infinite number of possible copy constructor signatures.

However, with one formal argument there are only four, corresponding to
the four possible combinations of having or not having 'volatile' and
'const'. Of these four, two have 'const', and two don't. As an example
of a copy constructor with non-const argument, check out std::auto_ptr.

 

[dragoncoder]

* dragoncoder:




A temporary (the actual argument) can't be bound to a reference to
non-const.

I am sorry I still don't get it. Where is the copy constructor
involved here? I am trying to call the constructor with the "const
char*" argument. Please explain.

 

[Alf P. Steinbach]

* dragoncoder:

I am sorry I still don't get it. Where is the copy constructor
involved here? I am trying to call the constructor with the "const
char*" argument. Please explain.

Except for possible optimization, which doesn't affect the requirements
on the string class, the statement

mystring s = "Nitin";

is equivalent to

mystring temp( "Nitin" );
mystring s( temp );

for some unique name 'temp'.

Repeat: even though just about any compiler optimizes away the
temporary, i.e. you don't get an actual copy constructor call, the copy
constructor must be available to do the job as if no optimization.

...

OK, lets go through the rest of the code, also.

=> cat mystring.h
#include<iostream>

Not necessary since iostreams aren't used in the header.

using namespace std;

Never have 'using namespace std;' in a header.

class mystring
{
char* p;
int capacity;
int len;
enum {DEF_SIZE = 100};

Reserve all uppercase identifiers for macros.

public:
// Constructors
mystring();
mystring(int size);

Naming: here you probably mean capacity, not size.

mystring(const char* str);

// Iniializer
void init(int size = DEF_SIZE);

Should not be public.

//Destructor
~mystring();

//Copy Constructor
mystring(mystring&);

Needs 'const' for the argument.

//Assignment Operator
mystring& operator = (mystring&);

Needs 'const' for the argument.

//Display String
void display();

OK as a debugging aid while developing the class, but after that, will
bind use of the class to some particular i/o environment, so ungood.

};

=> cat mystring.cpp
#include<iostream>
using namespace std;

#include "mystring.h"

It's a good idea to include the module header as the very first thing in
the implementation file (not after anything else, except comments), to
make more sure that it includes everything necessary.

//Constructors
mystring::mystring()
{
cout << "Default Constructor" << endl;
init();
len = 0;
}

mystring::mystring(int size)
{
cout << "Constructor having size" << endl;
init(size);
len = 0;
}

mystring::mystring(const char* str)
{
cout << "Constructor with string argument" << endl;
int length = strlen(str);
init(length + 1);
len = length + 1;

Although 'len' doesn't seem to be used for anything, if it's meant to
represent the length of the string then surely it should be 'length',
not 'length+1'.

strcpy(p, str);
}

void mystring::init(int size)
{
p = new char[size];
capacity = size;
}

mystring::~mystring()
{
cout << "Destructor" << endl;
delete p;
p = NULL;

Assignment of NULL unnecessary - the destructor call is the last thing
that ever happens with this object.

}
mystring::mystring(mystring& other)
{
cout << "Copy Constructor" << endl;
delete p;

p has not been initialized, so this is Undefined Behavior.

p = new char[strlen(other.p) + 1];
strcpy(p,other.p);

Fields of 'other' not copied.

}

mystring& mystring:perator=(mystring& other)
{
cout << "Assignment Operator" << endl;
if(this == &other)
return *this;
delete p;
p = new char[strlen(other.p)+1];

At this point you may get an exception. OK, it's not a practical
consideration, because when you run out of memory there's not much to do
except terminate. But as a general principle, strive to create
exception safe code -- here you'd leave the object with an invalid 'p'
member.

The 'swap' idiom for assignment helps with exception safe assignment.

 

[Thinking_In_CPP]

* dragoncoder:

I am sorry I still don't get it. Where is the copy constructor
involved here? I am trying to call the constructor with the "const
char*" argument. Please explain.

Except for possible optimization, which doesn't affect the requirements
on the string class, the statement

mystring s = "Nitin";

is equivalent to

mystring temp( "Nitin" );
mystring s( temp );

for some unique name 'temp'.

Repeat: even though just about any compiler optimizes away the
temporary, i.e. you don't get an actual copy constructor call, the copy
constructor must be available to do the job as if no optimization.

...

OK, lets go through the rest of the code, also.

=> cat mystring.h
#include<iostream>

Not necessary since iostreams aren't used in the header.

using namespace std;

Never have 'using namespace std;' in a header.

class mystring
{
char* p;
int capacity;
int len;
enum {DEF_SIZE = 100};

Reserve all uppercase identifiers for macros.

public:
// Constructors
mystring();
mystring(int size);

Naming: here you probably mean capacity, not size.

mystring(const char* str);

// Iniializer
void init(int size = DEF_SIZE);

Should not be public.

//Destructor
~mystring();

//Copy Constructor
mystring(mystring&);

Needs 'const' for the argument.

//Assignment Operator
mystring& operator = (mystring&);

Needs 'const' for the argument.

//Display String
void display();

OK as a debugging aid while developing the class, but after that, will
bind use of the class to some particular i/o environment, so ungood.

};

=> cat mystring.cpp
#include<iostream>
using namespace std;

#include "mystring.h"

It's a good idea to include the module header as the very first thing in
the implementation file (not after anything else, except comments), to
make more sure that it includes everything necessary.

//Constructors
mystring::mystring()
{
cout << "Default Constructor" << endl;
init();
len = 0;
}

mystring::mystring(int size)
{
cout << "Constructor having size" << endl;
init(size);
len = 0;
}

mystring::mystring(const char* str)
{
cout << "Constructor with string argument" << endl;
int length = strlen(str);
init(length + 1);
len = length + 1;

Although 'len' doesn't seem to be used for anything, if it's meant to
represent the length of the string then surely it should be 'length',
not 'length+1'.

strcpy(p, str);
}

void mystring::init(int size)
{
p = new char[size];
capacity = size;
}

mystring::~mystring()
{
cout << "Destructor" << endl;
delete p;
p = NULL;

Assignment of NULL unnecessary - the destructor call is the last thing
that ever happens with this object.

}
mystring::mystring(mystring& other)
{
cout << "Copy Constructor" << endl;
delete p;

p has not been initialized, so this is Undefined Behavior.

p = new char[strlen(other.p) + 1];
strcpy(p,other.p);

Fields of 'other' not copied.

}

mystring& mystring:perator=(mystring& other)
{
cout << "Assignment Operator" << endl;
if(this == &other)
return *this;
delete p;
p = new char[strlen(other.p)+1];

At this point you may get an exception. OK, it's not a practical
consideration, because when you run out of memory there's not much to do
except terminate. But as a general principle, strive to create
exception safe code -- here you'd leave the object with an invalid 'p'
member.

The 'swap' idiom for assignment helps with exception safe assignment.

strcpy(p,other.p);
return *this;
}

void mystring::display()
{
cout << *p << endl;
cout << "Length of string is " << len << endl;
cout << "Capacity of the string is " << capacity << endl;
}

=> cat str.cpp
#include<iostream>
using namespace std;
#include "mystring.h"

int main()
{
mystring s = "Nitin";
return 0;
}

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?- Hide quoted text -

- Show quoted text -- Hide quoted text -

- Show quoted text -

one more thing
use delete [] p instead of delete p

 

[Ron Natalie]

the form for a copy constructor uses the const key word.

Not necessarily. Any function that can take a reference
to a single argument of the type of the class is a copy constructor
regardless of the cv-qualification.

When you use
the = operator, it calls the copy constructor.

The = operator in his case does not use the copy constructor.

The = in the first line of his main function is NOT an operator.
It's the syntax for initialization.