How can I better manage strings and memory

Discussion in 'C++' started by DeveloperDave, Oct 19, 2008.

  1. 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
    DeveloperDave, Oct 19, 2008
    #1
    1. Advertising

  2. 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/breeze/>
    Do you need expertise in C++? I'm available.
    Gennaro Prota, Oct 19, 2008
    #2
    1. Advertising

  3. DeveloperDave

    red floyd Guest

    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?
    red floyd, Oct 19, 2008
    #3
  4. On Oct 19, 7:34 pm, red floyd <> 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.
    DeveloperDave, Oct 19, 2008
    #4
  5. DeveloperDave

    Guest

    On Oct 19, 12:29 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?
    >
    > 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
    , Oct 19, 2008
    #5
  6. DeveloperDave wrote:
    > On Oct 19, 7:34 pm, red floyd <> 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::eek: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.

    --
    Thomas
    Thomas J. Gritzan, Oct 19, 2008
    #6
  7. DeveloperDave

    Ian Collins Guest

    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
    Ian Collins, Oct 19, 2008
    #7
  8. DeveloperDave

    red floyd Guest

    DeveloperDave wrote:
    > On Oct 19, 7:34 pm, red floyd <> 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.
    red floyd, Oct 19, 2008
    #8
  9. DeveloperDave wrote:
    > On Oct 19, 7:34 pm, red floyd <> 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::eek: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.

    --
    Gennaro Prota | name.surname yahoo.com
    Breeze C++ (preview): <https://sourceforge.net/projects/breeze/>
    Do you need expertise in C++? I'm available.
    Gennaro Prota, Oct 19, 2008
    #9
  10. On Oct 19, 10:49 pm, Gennaro Prota <gennaro/> wrote:
    > DeveloperDave wrote:
    > > On Oct 19, 7:34 pm, red floyd <> 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::eek: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.
    >
    > --
    >    Gennaro Prota         |           name.surname yahoo.com
    >      Breeze C++ (preview): <https://sourceforge.net/projects/breeze/>
    >      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.
    DeveloperDave, Oct 19, 2008
    #10
  11. 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.
    Juha Nieminen, Oct 20, 2008
    #11
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Replies:
    11
    Views:
    477
    David Bolen
    Jul 19, 2006
  2. yezi

    How to manage the memory in C?

    yezi, Nov 21, 2005, in forum: C Programming
    Replies:
    5
    Views:
    317
    Richard Heathfield
    Nov 22, 2005
  3. Ben

    Strings, Strings and Damned Strings

    Ben, Jun 22, 2006, in forum: C Programming
    Replies:
    14
    Views:
    729
    Malcolm
    Jun 24, 2006
  4. Tobiah
    Replies:
    4
    Views:
    198
    Tobiah
    May 11, 2009
  5. Eric Kung
    Replies:
    1
    Views:
    540
    Alf P. Steinbach /Usenet
    Jul 8, 2010
Loading...

Share This Page