Skip to main content
gamedev.net
🔒 Locked

Problems returning string

Started by MajinMusashi 14 replies 700+ views
Original Post
MajinMusashi
MajinMusashi
Hello! CONTEXT: I'm using Visual C++ 6 and I'm trying to format some text (and return it). The text in question is the current time (hh:mm:ss format), that must be stored in the records file to keeps track of how much the user has played. PROBLEM: I've tried two different implementatios for the same method, in C and in C++ styles (none actually works :( ). Please, read the code below to a better understanding: 
// OBS 1 - "getText()" must return "const char*"
// OBS 2 - The "get" methods return integers

// C style
// The text is correctly formated here, but, as tempString goes out of scope,
//   "result" points to an undefined position, so the caller of "getText()"
//   will get garbage   
const char* C_Clock::getText() {
  char tempString[9];
  _snprintf( tempString, 9, "%02d:%02d:%02d", getHours(), getMinutes(), getSeconds() );
  const char* result = tempString;
  return result;
}

// C++ style
// This style suffers from the same thing as the C style, plus the fact that I
//   don't know how to force the values to be formated as "hh:mm:ss"
const char* C_Clock::getText() {
  std::ostringstream tempString;
  tempString << getHours() << ":" << getMinutes() << ":" << getSeconds() << std::ends;
  return tempString.str().c_str();
}
QUESTION: Could you point me the right way to go? It would be better yet if you point me BOTH ways (C and C++)... Thanks a lot! // lb edit - added a title
Sneftel
Sneftel
Why not return a std::string instead of a char*?
LessBread
LessBread 
const char* C_Clock::getText() {  char tempString[9];  _snprintf( tempString, 9, "%02d:%02d:%02d", getHours(), getMinutes(), getSeconds() );  const char* result = tempString;  return result;}


The variable tempString is a local variable so it loses validity once the function returns. You'll need to allocate it from the heap or reference a global variable instead. If you allocate the variable from the heap, don't forget to deallocate it when it's no longer used.
"I thought what I'd do was, I'd pretend I was one of those deaf-mutes." - the Laughing Man
MajinMusashi
MajinMusashi
Quote:
Original post by Sneftel
Why not return a std::string instead of a char*?


Yes, I could do that and use "c_str()" to pass the value to the XML library I'm using, but I'm really interest on how to solve the problem using "const char*".
Can you help me?
MajinMusashi
MajinMusashi
Quote:
The variable tempString is a local variable so it loses validity once the function returns. You'll need to allocate it from the heap or reference a global variable instead. If you allocate the variable from the heap, don't forget to deallocate it when it's no longer used.

Could you give some example? Is it not possible to do all the job (allocation and deallocation) inside "getText()" and return only the interesting thing to the caller?

Thanks a lot!

nilkn
nilkn
As LessBread said, tempString is a local stack variable and will be done away with when the function exits. However, you still set result to point at it. If you want to set result to point to tempString and have it work properly, you need to allocate tempString on the heap so it's not destroyed when the function exits.

Or, you could use Sneftel's expert advice of using std::string rather than const char*.
bjle
bjle
I don't think there's any real "good" way to return a char *. The way a some libraries do it, is they have you pass in a char * and its length, modify that, and return the same pointer you passed in. It's two extra parameters to send, but you get the functionality of returning a c-style string.
MajinMusashi
MajinMusashi
Quote:
Original post by nilkn
Or, you could use Sneftel's expert advice of using std::string rather than const char*.

I think this is the way to go. I'll forget the "const char*" thing for now ;)

smart_idiot
smart_idiot
The easy way to use a character pointer here is to make your buffer static, so that it isn't on the stack.

There are two problems with this however:

1) It isn't safe in a multithreading environment.
2) The result of a previous call shouldn't be used after the function is called again, as the new result will overwrite the old one.

You could also allocate memory to avoid those problems, but then you'd have to free it when you were done with it. If you write something natural like cout << clock.getText(), then you've just managed to create a memory leak.

You can avoid all those problems by just using std::string.
Chess is played by three people. Two people play the game; the third provides moral support for the pawns. The object of the game is to kill your opponent by flinging captured pieces at his head. Since the only piece that can be killed is a pawn, the two armies agree to meet in a pawn-infested area (or even a pawn shop) and kill as many pawns as possible in the crossfire. If the game goes on for an hour, one player may legally attempt to gouge out the other
MajinMusashi
MajinMusashi
Quote:
Original post by bjle
I don't think there's any real "good" way to return a char *. The way a some libraries do it, is they have you pass in a char * and its length, modify that, and return the same pointer you passed in. It's two extra parameters to send, but you get the functionality of returning a c-style string.

Similar to the "printf" family, right? Better go with std::string :)
Thanks!

MajinMusashi
MajinMusashi
Guys, could you help me on formating the text on the C++ style example (so I can completely get rid of C character arrays)? Otherwise I'll end up with a C and C++ hibrid solution :(

Thanks a lot!
smart_idiot
smart_idiot 
// Example #1#include <string>std::string C_Clock::getText() {  char tempString[9];  _snprintf(tempString, 9, "%02d:%02d:%02d", getHours(), getMinutes(), getSeconds());  return tempString;}


// Example #2#include <string>#include <sstream>std::string C_Clock::getText() {  std::stringstream stream;  stream.fill('0');  stream.width(2);  stream << getHours() << ':';  stream.width(2);  stream << getMinutes() << ':';  stream.width(2);  stream << getSeconds();  return stream.str();}
Chess is played by three people. Two people play the game; the third provides moral support for the pawns. The object of the game is to kill your opponent by flinging captured pieces at his head. Since the only piece that can be killed is a pawn, the two armies agree to meet in a pawn-infested area (or even a pawn shop) and kill as many pawns as possible in the crossfire. If the game goes on for an hour, one player may legally attempt to gouge out the other
MajinMusashi
MajinMusashi
Quote:
Original post by smart_idiot
*** Source Snippet Removed ***


Wow, thanks a lot, Idiot ;) I think I'll stay with the #1 solution.

LessBread
LessBread
Quote:
Original post by MajinMusashi
Could you give some example? Is it not possible to do all the job (allocation and deallocation) inside "getText()" and return only the interesting thing to the caller?


What I would have done is probably made the string variable (tempString) a member of the clock class, named "displayText" or similar. This way the memory for the string would be allocated as part of the object and thus deallocated when object is destroyed. And that would make getText more of an update method, updating the text in the display string when called.
"I thought what I'd do was, I'd pretend I was one of those deaf-mutes." - the Laughing Man
MajinMusashi
MajinMusashi
Quote:
Original post by LessBread
What I would have done is probably made the string variable (tempString) a member of the clock class, named "displayText" or similar. This way the memory for the string would be allocated as part of the object and thus deallocated when object is destroyed. And that would make getText more of an update method, updating the text in the display string when called.

Agreed, LessBread. The resulting code now is:
// displayText is now a member of the clock class (char displayText[9])const char* C_Clock::getText() {  _snprintf( displayText, 9, "%02d:%02d:%02d", getHours(), getMinutes(), getSeconds() );  return displayText;}


Thanks a lot!

Zahlman
Zahlman
Do clients of the class really need to be able to check hours/minutes/seconds directly? I would assume that normally one only wants to tell a clock to advance/retreat, and then check the current time as a whole. Accessors don't really help internally and one shouldn't add more to external interfaces than necessary.

Also, for outputting stuff, the C++ idiom is to accept a stream reference and return it back, rather than implementing a toString() (as Java would call it):

ostream& Clock::display(ostream& os) const {  // Another useful thing is knowing how to do the formatting 'manually'  return os << hours / 10 << hours % 10 << ':'             << minutes / 10 << minutes % 10 << ':'            << seconds / 10 << seconds % 10;}ostream& operator<<(ostream& os, const Clock& c) {  return c.display(os);}// Now you can do:Clock noon(12, 0, 0);noon.display(cout);// Or more idiomatically:cout << noon;


Oh, and std::ends is not called for when using the current standard library version of stringstream (it is in , NOT , or anything else).

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.