Sign in to follow this  

function returning a string

This topic is 3456 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I have a little code design question. Throughout my engine, every method that returns a string, have done so by either returning a "const char*" or a "const wchar_t*" which points to a string object. For example, a texture object returning its image's filename: const char* CTexture::GetFilename() All my string objects are from my own string class, which internally stores the string as a null terminated char array. Now I'm changing everything to be more standard C++. So I'm replacing my string class with std::string. Now the question is how should I return a string now that I'm using std::string. I've searched around and it seems that returning a pointer from std::string.c_str() is a really bad thing. So what I've come up with so far is: 1. std::string CTexture::GetFilename() 2. void CTexture::GetFilename( std::string& pFilename ) I like the first one better because it makes it easier to pass the string to another function like: PrintStuff( texture.GetFilename() ); But it does also make a copy of the string which often seems unneccessary. The second solution looks nicer but is quite annoying when trying to pass the string along. Any ideas?

Share this post


Link to post
Share on other sites
Yes, you should use:
std::string CTexture::GetFilename()

The copy isn't problematic because they use a shadow copy.

Share this post


Link to post
Share on other sites
"Shadow Copy", does this mean that the new string internally points to the old one until either of them changes? I'm new to std::string and not really familiar with how it works internally.

Share this post


Link to post
Share on other sites
The compiler will usually apply either RVO or NRVO to your function, thereby making option 1 work as if you had written option 2.

Shadow copy (which I suspect is used here to refer to COW semantics, copy-on-write) indeed means that a single piece of data is referenced until it's modified. However, most recent versions don't use it, because it creates performance losses due to thread safety.

Share this post


Link to post
Share on other sites
Another somewhat related question: What if a function needs a string as a parameter should I use:

1. PrintStuff( const char* pStr )
2. PrintStuff( const string& pStr )
3. PrintStuff( string pStr )

Currently I have the first option everywhere. Maybe I should change to option 2 and get rid of all the char pointers in the code. I'm guessing option 2 is better than option 3, or is there some reason for using the third option?

Share this post


Link to post
Share on other sites
option 2 is best, if you only perform read access on pStr, and passing by reference will reduce copy overhead.

Share this post


Link to post
Share on other sites
1. It is not always possible to compile version 1. as version 2. (think for example when GetFilename() has multiple return path, or what happens if the return value has a static counter in the constructor?).

2. A drawback of version 2. is that it is not clear if the input argument will be used as input and output or only as output, looking only at the declaration.

3. A third option is to return a std::auto_ptr<std::string>.

Share this post


Link to post
Share on other sites
Since no concrete example for reason of getFilename to exist in the first place, here's an alternate way of thinking.

Why would you need to have getFilename()?

Printing it out?
// friend function
std::ostream & operator<<(std::ostream & os, const Texture & t) {
return os << t.filename;
}


Putting it in hash table?
TextureManager::loadTexture(std::string & s) {
TexturePtr t(new Texture(s)); // throws by RAII
my_lookup.insert(s, t);
}
// lookup is now trivial


And there's other ways to preserve encapsulation. So what are the real use cases? Why do you need getFilename()?

Share this post


Link to post
Share on other sites
I use a GetSomeKindOfString() in many places, like for example my GUI system which uses it to query object names or getting text from a control.

Share this post


Link to post
Share on other sites
Quote:
Original post by Decept
Another somewhat related question: What if a function needs a string as a parameter should I use:

1. PrintStuff( const char* pStr )
2. PrintStuff( const string& pStr )
3. PrintStuff( string pStr )

Currently I have the first option everywhere. Maybe I should change to option 2 and get rid of all the char pointers in the code. I'm guessing option 2 is better than option 3, or is there some reason for using the third option?


There *can* be cases where you want to use the third option. It copies the string, which is usually a waste of time. But if you need a copy of the string inside the function anyway, this will obviously be the cleanest way to do it. (The alternative would be to take a string reference, and then inside the function create your temporary copy)

But yeah, normally you should prefer option 2. Since you're just passing a reference to the string, no copies are made, so it should be just as efficient as #1, while retaining all the niceness of "proper" strings.

Share this post


Link to post
Share on other sites
Quote:
std::string CTexture::GetFilename()

Return a const string instead. That way this wont be allowed.
texture.GetFilename() = "fredandbarney.png";


Quote:
It should probably be named rStr since it is a reference and not a pointer.
p is probably parameter, and not pointer as both the pass-by-value, pass-by-pointer and pass-by-reference is using it.

Shadow-copy/COW probably won't be needed if you use const references(thinking of gotw88)

Quote:
2. A drawback of version 2. is that it is not clear if the input argument will be used as input and output or only as output, looking only at the declaration.
It's accepting a const reference, how would you modify it in a legal way? Arguments that are to be modified, if used, I pass as a non const pointer often wrapped in a NonNull<> struct.

Quote:
3. A third option is to return a std::auto_ptr<std::string>.
please don't. Your dynamically adding memory and each call must check that the returned value is non-null.

Share this post


Link to post
Share on other sites
Quote:
Original post by sirGustav
Quote:
std::string CTexture::GetFilename()

Return a const string instead. That way this wont be allowed.
texture.GetFilename() = "fredandbarney.png";


That only matters when you return references. Which is not the case here. Even without const I don't think your example would compile because texture.GetFilename() is not an lvalue expression.

Share this post


Link to post
Share on other sites
Sign in to follow this