How can I better manage strings and memory

[DeveloperDave]

Hi,

I am trying to improve some code that I currently have.

I have a simple class called RequestMessage e.g.

class RequestMessage
{
public:
RequestMessage();
~RequestMessage();

string& getMessage()

private:
string origin
string type;
string body;
....
....
string rqstMessage;
};

getMessage formats the private members and stores them in rqstMessage
before returning a reference to it.

string&
RequestMessage::getMessage()
{
ostringstream* message = new ostringstream("Orgin: ");
message << origin << endl;
message << "Type:" << type << endl;
message << "Body: << body << endl;

rqstMessage = message->str();

return rqstMessage;
}

The problem I have, is that I don't like the fact I already have
stored my data in strings, and then use another string to store it all
again in a formatted way.
Is there a better way to do this, without using the additional memory?

Cheers

 

[Gennaro Prota]

Hi,

I am trying to improve some code that I currently have.

I have a simple class called RequestMessage e.g.

class RequestMessage
{
public:
RequestMessage();
~RequestMessage();

string& getMessage()

private:
string origin
string type;
string body;
....
....
string rqstMessage;
};

getMessage formats the private members and stores them in rqstMessage
before returning a reference to it.

string&
RequestMessage::getMessage()
{
ostringstream* message = new ostringstream("Orgin: ");
message << origin << endl;
message << "Type:" << type << endl;
message << "Body: << body << endl;

rqstMessage = message->str();

return rqstMessage;
}

The problem I have, is that I don't like the fact I already have
stored my data in strings, and then use another string to store it all
again in a formatted way.
Is there a better way to do this, without using the additional memory?

It would be important to know the context. Anyhow, first be sure
you don't leak the "additional memory" for the ostringstream
(this is not Java). Then, what's the role of rqstMessage? Is it
a cache? Did you make measurements that show you *need* it? If
so make it a mutable member and make getMessage() const.
Otherwise just forget about it, and return a string instead of a
reference.

 

[red floyd]

Hi,

I am trying to improve some code that I currently have.

I have a simple class called RequestMessage e.g.

class RequestMessage
{
public:
RequestMessage();
~RequestMessage();

string& getMessage()

private:
string origin
string type;
string body;
....
....
string rqstMessage;
};

getMessage formats the private members and stores them in rqstMessage
before returning a reference to it.

string&
RequestMessage::getMessage()
{
ostringstream* message = new ostringstream("Orgin: ");
message << origin << endl;
message << "Type:" << type << endl;
message << "Body: << body << endl;

rqstMessage = message->str();

return rqstMessage;
}

This is a memory leak. Why are you newing message? And why are you not
deleteing it? It should simply be declared as a local variable:

ostringstream message;

Let me guess, you're coming from Java?

 

[DeveloperDave]

This is a memory leak.  Why are you newing message?  And why are you not
deleteing it?  It should simply be declared as a local variable:

   ostringstream message;

Let me guess, you're coming from Java?

Guys, I'm not coming from java, and I don't want to know that I should
always call delete with new (I know this already). This also isn't
the final code, it is an example to illustrate my problem. Why not
comment that the "..." will cause the code not to compile as well?

The question is:

is it possible to use the member variables to construct a formatted
string, i.e.
Something like

"Orgin:%s\nType:%s\nBody:%s\n"

Replacing the %s with my member variables, "orgin", "type", and
"body", but without storing these in a new string i.e. not storing the
same information twice.

 

[AnonMail2005]

Hi,

I am trying to improve some code that I currently have.

I have a simple class called RequestMessage e.g.

class RequestMessage
{
public:
    RequestMessage();
    ~RequestMessage();

   string& getMessage()

private:
   string origin
   string type;
   string body;
   ....
   ....
  string rqstMessage;

};

getMessage formats the private members and stores them in rqstMessage
before returning a reference to it.

string&
RequestMessage::getMessage()
{
   ostringstream* message = new ostringstream("Orgin: ");
   message << origin << endl;
   message << "Type:" << type << endl;
   message << "Body:  << body << endl;

   rqstMessage =  message->str();

   return rqstMessage;

}

The problem I have, is that I don't like the fact I already have
stored my data in strings, and then use another string to store it all
again in a formatted way.
Is there a better way to do this, without using the additional memory?

Cheers

Without context, it's hard to say. But why not build the message
in a constructor so that the arguments to the contructor are the
components and only one copy of the built message is kept within
the class.

HTH

 

[Thomas J. Gritzan]

Guys, I'm not coming from java, and I don't want to know that I should
always call delete with new (I know this already). This also isn't
the final code, it is an example to illustrate my problem. Why not
comment that the "..." will cause the code not to compile as well?

The point wasn't to use delete, but not using 'new' at all for a
variable that is only locally used. If you 'new' the ostringstream,
there's no point in avoiding the string copy.

The question is:

is it possible to use the member variables to construct a formatted
string, i.e.
Something like

"Orgin:%s\nType:%s\nBody:%s\n"
Yes,

Replacing the %s with my member variables, "orgin", "type", and
"body", but without storing these in a new string i.e. not storing the
same information twice.

Without storing the complete string? That depends on what you want to do
with the string.

For only outputting it to console (or file), you could implement a print
function that outputs to ostream&:

RequestMessage:rint(std:stream& message) const
{
message << origin << '\n'
<< "Type:" << type << '\n'
<< "Body:" << body << endl;
}

That way, a client could pretty print the object to cout, to a file
stream, a memory stream or any other custom stream.

 

[Ian Collins]

The question is:

is it possible to use the member variables to construct a formatted
string, i.e.
Something like

"Orgin:%s\nType:%s\nBody:%s\n"

Replacing the %s with my member variables, "orgin", "type", and
"body", but without storing these in a new string i.e. not storing the
same information twice.

That all depends on what you want to do with the result. If you just
want to output it, then follow Thomas' advice. But don't forget the
data probably will be stored twice because the output stream is buffered!

 

[red floyd]

Guys, I'm not coming from java, and I don't want to know that I should
always call delete with new (I know this already). This also isn't
the final code, it is an example to illustrate my problem. Why not
comment that the "..." will cause the code not to compile as well?

Frankly, it doesn't matter. Your code flat out will not work.

ostreamstring *ss = new ostreamstring("origin: ");
ss << "Hello";

Will *NOT* work. There is no operator<< defined which takes a
ostreamstring* as a left hand side.

 

[Gennaro Prota]

[...]
The question is:

is it possible to use the member variables to construct a formatted
string, i.e.
Something like

"Orgin:%s\nType:%s\nBody:%s\n"

Replacing the %s with my member variables, "orgin", "type", and
"body", but without storing these in a new string i.e. not storing the
same information twice.

You don't need to store the result into a data member, at all.
For most purposes, outputting to a stream is the best solution,
which you can do as suggested by Thomas or, more idiomatically,
writing your own stream inserter (operator <<). Of course, too,
the inserter may just forward the work to print(std:stream &).
In short, if you are formatting then use the C++ idioms for
formatting.

Otherwise, for the simple need you ask about, you can just
concatenate:

// note: not compiled
std::string
RequestMessage::getMessage() const
{
return "Orgin:" /*[sic]*/ + m_origin + "\n"
+ "Type:" + m_type + "\n"
+ "Body:" + m_body + "\n"
;
}

It's not an approach which defies the centuries but it's an
approach

If this doesn't answer your question you have to give us more
details.

 

[DeveloperDave]

DeveloperDave wrote:
Hi,
I am trying to improve some code that I currently have.
I have a simple class called RequestMessage e.g.
class RequestMessage
{
public:
    RequestMessage();
    ~RequestMessage();
   string& getMessage()
private:
   string origin
   string type;
   string body;
   ....
   ....
  string rqstMessage;
};
getMessage formats the private members and stores them in rqstMessage
before returning a reference to it.
string&
RequestMessage::getMessage()
{
   ostringstream* message = new ostringstream("Orgin: ");
   message << origin << endl;
   message << "Type:" << type << endl;
   message << "Body:  << body << endl;
   rqstMessage =  message->str();
   return rqstMessage;
}

     [...]
The question is:

is it possible to use the member variables to construct a formatted
string, i.e.
Something like

Replacing the %s with my member variables, "orgin", "type", and
"body", but without storing these in a new string i.e. not storing the
same information twice.

You don't need to store the result into a data member, at all.
For most purposes, outputting to a stream is the best solution,
which you can do as suggested by Thomas or, more idiomatically,
writing your own stream inserter (operator <<). Of course, too,
the inserter may just forward the work to print(std:stream &).
In short, if you are formatting then use the C++ idioms for
formatting.

Otherwise, for the simple need you ask about, you can just
concatenate:

   // note: not compiled
   std::string
   RequestMessage::getMessage() const
   {
       return "Orgin:" /*[sic]*/ + m_origin + "\n"
              + "Type:"          + m_type   + "\n"
              + "Body:"          + m_body   + "\n"
              ;
   }

It's not an approach which defies the centuries but it's an
approach

If this doesn't answer your question you have to give us more
details.

Thanks for the constructive suggestions. I will eventually be writing
the data to a stream, so I guess passing that stream in writing to it,
is the best approach.
Cheers.

 

[Juha Nieminen]

Guys, I'm not coming from java, and I don't want to know that I should
always call delete with new (I know this already). This also isn't
the final code, it is an example to illustrate my problem. Why not
comment that the "..." will cause the code not to compile as well?

I don't think you get the point. The fact that you used 'new' in a
place where it's not needed (and in fact wrong) shows that you have a
bad approach at C++ programming. It's imperative to get rid of these bad
habits if you want to become a good C++ programmer.

There are many reasons why you should avoid 'new' in a situation like
you wrote:

- Allocating a local object with 'new' is usually much less efficient
than instantiating it directly. Stack-allocation is usually much faster
and efficient than heap-allocation.

- When you allocate something with 'new' and handle it with a raw
pointer, you always have the risk of a memory leak, even if you
seemingly 'delete' the object afterwards. Functions can have surprising
hidden exit points. C++ does not have garbage collection, so you have to
learn to live with this.

- Your code doesn't even work. You are calling "message << something"
even though 'message' is a *pointer*, not an ostringstream object. You
can't apply the << operator to a pointer.