How to save a string to a string* member of a class in C++?

Viewed 204

I have a C++ class which contains in its members a std::string*. I want to make a constructor for this class, which takes a std::string as an arguement and makes the string pointer to point at it:

class foo
{
private:
    std::string * bar;
public:
    foo(std::string);
}
foo::foo(std::string s)
{
    //code
}

If I do this->bar = &s; it obviously doesn't work (saves the address of the s string, as it should). How do I go about doing this?

I tried creating a constructor which takes a string pointer as its arguement but it also didn't work properly, I assume for similar reasons.

Edit: Upon reading the comments I decided to alter the constructor so it now takes a std::string* as its arguement, and work from there forward.

2 Answers

You could write your constructor as follows:

foo::foo(std::string s)
{
    bar = new std::string(s);  // code
}

But now you have a memory leak, so you need a destructor:

foo::~foo() { delete bar; }

And then you need to implement or disable copy/assignment.

class foo final
{
   ...
   foo(const foo&) = delete;
   foo& operator=(const foo&) = delete;
   ...
 };

That's all a lot of effort to store a pointer to std::string instead of an instance:

class foo final
{
    std::string bar;  // NOT std::string* bar
public:
    foo(std::string s) : bar(s) {}
};

If you really want a pointer, it would be much better to use a std::unique_ptr or std::shared_ptr.

class foo final
{
    std::unique_ptr<std::string> bar;
public:
    foo(std::string s) : bar(std::make_unique<std::string>(s)) {}
};

std::unique_ptr can't be copied/assigned, so you don't even have to disable copy/assignment for foo, although you may want to for better error messages.

If it is absolutely necessary to have a pointer to a string - make sure to acquire an ownership to that string (by making an internal copy of it first, for example). Than initialize your pointer with an address of that copy.

class foo
{
private:
    std::string * bar;
    std::string redundantCopyOfTheString;
public:
    foo(std::string);
}
foo::foo(std::string s):
   redundantCopyOfTheString(s),
   bar(&redundantCopyOfTheString)
{
    //code
}

However, make sure that you really need this pointer as a member of the class at the first place. It looks like a bad design.

I suggest you to describe a problem that brought you to this pointer-based solution, and post as a separate question, with a request to improve the design.

Related