Asp Forum - How can I better manage strings and memory

DeveloperDave

10/19/2008 4:29:00 PM

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

16 Answers

Gennaro Prota

10/19/2008 4:47:00 PM

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 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.

--
Gennaro Prota | name.surname yahoo.com
Breeze C++ (preview): <https://sourceforge.net/projects/b...
Do you need expertise in C++? I'm available.

red floyd

10/19/2008 6:35:00 PM

DeveloperDave

10/19/2008 7:39:00 PM

On Oct 19, 7:34 pm, red floyd <no.spam.h...@example.com> wrote:
> 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;
> > }
>
> 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@gmail.com

10/19/2008 7:51:00 PM

On Oct 19, 12:29 pm, DeveloperDave <davej2...@gmail.com> 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 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

10/19/2008 8:23:00 PM

DeveloperDave wrote:
> On Oct 19, 7:34 pm, red floyd <no.spam.h...@example.com> wrote:
>> 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 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::print(std::ostream& 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.

--
Thomas

Ian Collins

10/19/2008 8:31:00 PM

DeveloperDave wrote:
>
> 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!

--
Ian Collins

red floyd

10/19/2008 9:08:00 PM

DeveloperDave wrote:
> On Oct 19, 7:34 pm, red floyd <no.spam.h...@example.com> wrote:
>> 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;
>>> }
>> 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?
>
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

10/19/2008 9:50:00 PM

DeveloperDave wrote:
> On Oct 19, 7:34 pm, red floyd <no.spam.h...@example.com> wrote:
>> 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
>
> "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::ostream &).
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.

--
Gennaro Prota | name.surname yahoo.com
Breeze C++ (preview): <https://sourceforge.net/projects/b...
Do you need expertise in C++? I'm available.

DeveloperDave

10/19/2008 10:25:00 PM

On Oct 19, 10:49 pm, Gennaro Prota <gennaro/pr...@yahoo.com> wrote:
> DeveloperDave wrote:
> > On Oct 19, 7:34 pm, red floyd <no.spam.h...@example.com> wrote:
> >> 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
>
> > "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::ostream &).
> 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.
>
> --
> Gennaro Prota | name.surname yahoo.com
> Breeze C++ (preview): <https://sourceforge.net/projects/b...
> Do you need expertise in C++? I'm available.

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

10/20/2008 3:59:00 PM

DeveloperDave wrote:
> 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.

comp.lang.c++

How can I better manage strings and memory

DeveloperDave

Gennaro Prota

red floyd

DeveloperDave

AnonMail2005@gmail.com

Thomas J. Gritzan

Ian Collins

red floyd

Gennaro Prota

DeveloperDave

Juha Nieminen

x Login to ForumsZone