Reputation: 21
I'm learning c++ and found a problem that I dont understand. I have this simple code:
#include <iostream>
#include <vector>
class Person
{
string * name;
public:
Person(const string & n) : name {new string {n}} {}
~Person() {delete name;}
string getName() const {return *name;}
};
int main()
{
vector<Person> people;
people.push_back(Person("Tom"));
cout << "Name is: " << people.back().getName() << endl;
return 0;
}
When I run it, I have no output. Dont know why? However, when I do simillar, but without vector everything is ok:
int main()
{
Person tom {"Tom"};
cout << "Name is: " << tom.getName() << endl;
return 0;
}
Upvotes: 1
Views: 109
Reputation:
The reason why your code is buggy has already been explained, but if you have C++11, you can use emplace_back
:
people.emplace_back("Tom");
Still, using pointers instead of a plain member variable is just unnecessarily complicating your program. The less memory management you have to do, the better. Read about the Rule of Zero. Better yet, if your getName()
function doesn't do anything special, remove it and just make name
public.
Upvotes: 1
Reputation: 1028
You are using wrong type. Your string
is already a string type. Write the code like this.
class Person
{
public:
Person (const string& n) : name(n) { }
~Person() {}
string getName() const { return name; }
private:
string name;
};
If you insist to use pointer in your member variable, you shall overwrite the copy constructor and overload assign operator.
Give you an example:
class Person
{
public:
Person (const char* n) : name(new char[strlen(n)+1]) { strcpy(name, n); }
Person (const Person& p) : name(new char[strlen(p.name)+1]) { strcpy(name, p.name); }
~Person() { delete [] name; }
Person& operator=(const Person& p)
{
if ( &p == this ) return *this;
delete [] name;
name = new char[strlen(p.name)+1];
strcpy(name, p.name);
return *this;
}
string getName() const { return name; }
private:
char* name;
};
Upvotes: 3
Reputation: 238081
As other said, better to do without pointers. However, if you wondering what is happening, the reason for what you get is that in this line people.push_back(Person("Tom"));
Person object is created, and its copy passed to vector. However, once the object is copied, destructor is executed which deletes the string.
With your usage of pointers, both the original Person object and its copy point to the same string in memory. String gets deleted by destructor, and name
pointer in the copy does not point to anything. Thus you get undefined behaviour.
To rectify this issue, either dont use pointers, or you need to define your own copy constructor. For example:
class Person
{
string * name;
public:
Person(const string & n) : name {new string {n}} {}
// copy constructor which makes new string in memory
//based on the original string.
Person(const Person & other) {
name = new string(other.getName());
}
~Person() { delete name; }
string getName() const {return *name;}
};
Upvotes: 9