String processing and memory management in C++

Question

I have certain string processing to do , the approach I am using is in following sample -

Void ProcessObjects(int nObject)
{
    std::string sInfostr;

    for(int i = 0;i<nObject;i++)
    {
      InfoObject Inf = new InfoObject; 
      GetInfoObject(&Inf);
      GetStoredInformation(Inf, std::string &sInfostr)
      delete Inf;           
    }
}


void GetStoredInformation(InfoObject Inf, std::string &sInfostr) 
{
    char tag[1000]; 

    GetInformation(&Inf);

    sprintf(tag, "name=%s",Inf.name);
    sInfostr += tag;        
    sprintf(tag, "name1=%s",Inf.name1);
    sInfostr += tag;
    sprintf(tag, "name2=%s",Inf.name2);
    sInfostr += tag;
    sprintf(tag, "name3=%s",Inf.name3);
    sInfostr += tag;
    sprintf(tag, "name4=%s",Inf.name4);
}

Now can I get some suggestion is it a good way to process string? Will I code go in any trouble if "nObject" above 10,000?

Why aren't GetInfObject() and GetStoredInformation() methods of InfoObject? — trojanfoe
– trojanfoe, Commented Oct 10, 2011 at 8:35
Why not declare name, name1.... name4 all as std::string? — Sarfaraz Nawaz
– Sarfaraz Nawaz, Commented Oct 10, 2011 at 8:37
"the approach I am using is in following sample". No it is not. This code is littered with syntax errors. Please post a working sample. — johnsyweb
– johnsyweb, Commented Oct 10, 2011 at 8:56
newing and deleting an object in the loop seems a bit unnecessary. You could just as well use an automatic object, possible hoisted out of the loop. - Also usage like InfoObject Inf = GetInfoObject(); is simpler and probably also more efficient than what you are doing. — visitor
– visitor, Commented Oct 10, 2011 at 9:34

Constantinius · Accepted Answer · 2011-10-10 08:37:28Z

1

I suggest you use std::istringstream instead of the old C-ish sprintf:

void GetStoredInformation(InfoObject Inf, std::string &sInfostr) 
{
    GetInformation(&Inf);
    std::istringstream stream();
    stream << "name=" << Inf.name
           << "name1=" << Inf.name1
           << "name2=" << Inf.name2
           << "name3=" << Inf.name3
           << "name4=" << Inf.name4;
    sInfostr = stream.str();
}

Will I code go in any trouble if "nObject" above 10,000?

Depends on your used hardware, from the C++ point of view it should not be a problem, unless your InfoObject is not extremely large.

answered Oct 10, 2011 at 8:37

Constantinius

35.2k9 gold badges79 silver badges85 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Björn Pollex · Accepted Answer · 2011-10-10 08:37:54Z

1

Your code is very sketchy, and contains several syntax errors and probably a memory leak.

About your actual question, using std::stringstream is the recommended way to build strings. It would look like this:

std::stringstream buffer;
buffer << "name=" << Inf.name
    << "name1=" << Inf.name1; // etc.

You can access the contents of buffer as an std::string using buffer.str().

Aside from being more elegant, this is probably also faster, because std::stringstream uses a smarter allocation-strategy than operator += of std::string.

answered Oct 10, 2011 at 8:37

Björn Pollex

77.1k30 gold badges206 silver badges290 bronze badges

2 Comments

James Kanze Over a year ago

Are you sure. The last time I measured (some time ago), the implementation of std::ostringstream I used ended up just doing += on a std::string. In this particular case (just appending strings), std::string would be simpler and more rapid. (Use ostringstream, of course, if you have to convert anything.)

Björn Pollex Over a year ago

@James: To be honest, I am not sure, since I have not measured.

Sarfaraz Nawaz · Accepted Answer · 2011-10-10 08:46:08Z

If you declare name, name1.... name4 all as std::string, then you would simply write:

sInfostr = "name  = " + Inf.name 
         + "name1 = " + Inf.name1 
         + "name2 = " + Inf.name2
         + "name3 = " + Inf.name3 
         + "name4 = " + Inf.name4;

which is much neater solution, as it is easier to read and maintain.

You can add member function called ToString() to InfoObject class as:

class InfoObject
{
  //...
  std::string ToString() const
  {
     return  = "name  = " + Inf.name 
             + "name1 = " + Inf.name1 
             + "name2 = " + Inf.name2
             + "name3 = " + Inf.name3 
             + "name4 = " + Inf.name4;
  }
};

Then GetStoredInformation would be just two lines:

void GetStoredInformation(InfoObject Inf, std::string &sInfostr) 
{
    GetInformation(&Inf);
    sInfostr = Inf.ToString();
}

Even better would be if you make GetStoredInformation a member function of InfoObject.

tenfour · Accepted Answer · 2011-10-10 08:40:25Z

0

You should not use sprintf at all. As you suspect, it is not safe. If any of the Inf.name* strings are longer than 1000 characters, you will corrupt the stack which can cause a crash or worse, a security breach via code injection.

Your format string is simple enough; the solution should be more like,

void GetStoredInformation(InfoObject Inf, std::string &sInfostr) 
{
    GetInformation(&Inf);
    sInfostr += "name=";// did you mean sInfostr = "name"?
    sInfostr += Inf.name;
    sInfostr += "name1=";
    sInfostr += Inf.name1;
    sInfostr += "name2=";
    sInfostr += Inf.name2;
    sInfostr += "name3=";
    sInfostr += Inf.name3;
    sInfostr += "name4=";
    sInfostr += Inf.name4;
}

answered Oct 10, 2011 at 8:40

tenfour

37k15 gold badges89 silver badges147 bronze badges

Collectives™ on Stack Overflow

String processing and memory management in C++

4 Answers 4

Comments

2 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

2 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related