homegrown string class optimisation

[bob]

Hi,

I'm looking at a legacy string class thats been in use here for a while
and I'd like to check out any options available to optimise it. I see a
couple of constructors that look dubious. Consider the following ctor.
It constructs a TtkString object with a string value of the integer
contained withing. e.g

TtkString one(456);
cout << one << endl;

prints;

456

// "string" is declared as a const char* within the class
TtkString::TtkString(int i)
{

std::stringstream s;
s << i << std::ends;
std::string myString = s.str();
const char* localString = myString.c_str();
int size(strlen(localString));
string = (char *) malloc((size + 1)*sizeof(char)) ;
memset(string,0,size+1);
strncpy(string, localString,size);

}

reading the code below I see that stringstreams are used (which seems
to me to be a bit heavyweight) and in addition a std::string is
constructed just to get the resulting const char*, subsequently the
malloc is done followed by a memset and a strcpy. It all seems a little
heavy to me (but I stand open to correction...perhaps this is not such
a bad approach altogether).
I was considering using something like this for the body of the same
function....

string = (char*) calloc (1, 33); // 32 bit system assumed.
memset(string,0,33);
itoa(i, string, 10);
however this is not working....Ive obviously messed something up. Can
anybody shed some light? My approach allocates 33 bytes regardless of
what the "i" argument is... e.g if its 1 then I don't need all 33
bytes, do I really?

If there are examples of this implemented in some library such as boost
or whatever, I'd be keen to check them out to see how they do it and
where my mistake is.

Finally, this constructor is overloaded to take floats, doubles , longs
etc and they all work more or less on the same approach. If I can
optimise this, I can optimise them all.

thanks for any assistance/input.

have a nice day.

G

[Greg]

bo*@blah.com wrote:

Hi,

I'm looking at a legacy string class thats been in use here for a while
and I'd like to check out any options available to optimise it. I see a
couple of constructors that look dubious. Consider the following ctor.
It constructs a TtkString object with a string value of the integer
contained withing. e.g

TtkString one(456);
cout << one << endl;

prints;

456

Constructors that accept a single, int parameter are usually best
declared "explicit". Otherwise, the implicit conversions - especially
from 0 - can be unexpected.

// "string" is declared as a const char* within the class
TtkString::TtkString(int i)
{

std::stringstream s;
s << i << std::ends;
std::string myString = s.str();
const char* localString = myString.c_str();
int size(strlen(localString));
string = (char *) malloc((size + 1)*sizeof(char)) ;
memset(string,0,size+1);
strncpy(string, localString,size);

}

First, "string" is poor choice for a member name - especially of a
string class that uses std::string's to some extent. So I would
redeclare the member variable to be a std::string and give it a
different name.

Now concerning the current implementation: this constructor starts out
OK. Granted, stringstream is a bit heavyweight. On the other hand, C++
is not blessed with an over abundance of convenient routines for
converting between numbers and strings. And none other than Bjarne
himself recommends using stringstream for this purpose. Now, I would be
much more concerned about the sudden, nightmarish turn for the worse
that the constructor takes, managing to call malloc(), memset(),
strncpy() - a veritable rogue's gallery of C's unsized, untyped
operations that have no business threatening our C++ code.

I was considering using something like this for the body of the same
function....

string = (char*) calloc (1, 33); // 32 bit system assumed.
memset(string,0,33);
itoa(i, string, 10);

First, itoa() is a non-standard routine. Furthermore, since calloc()
returns zero-initialized memory there is no point in zeroing out the
memory a second time. And what is the rationale for the magic number
33? Generally choosing a power of two would make a lot more sense given
that computers are binary machines. Besidss, a 33 digit number is a bit
excessive. I am not sure whether even a 128-bit long double has that
many digits of precision.

I would just stick with the std::stringstream and copy its std::string
to a std::string member variable (replacing the const char pointer) as
mentioned above. If you do decide to replace stringstream, then I would
use a standard routine with a sized, character buffer, such as
snprintf(), and then copy the buffer into a std::string.

however this is not working....Ive obviously messed something up. Can
anybody shed some light? My approach allocates 33 bytes regardless of
what the "i" argument is... e.g if its 1 then I don't need all 33
bytes, do I really?

The best idea is to delegate memory handling chores to a class object
like std::string. There is no other change worth making until all of
the calls to malloc, memset, memcpy and their ilk have been eliminated
by one means or another.

Greg

[Daniel T.]

"bo*@blah.com" <Gr**********@gmail.comwrote:

I'm looking at a legacy string class thats been in use here for a while
and I'd like to check out any options available to optimise it. I see a
couple of constructors that look dubious. Consider the following ctor.
It constructs a TtkString object with a string value of the integer
contained withing. e.g

TtkString one(456);
cout << one << endl;

prints;

456

// "string" is declared as a const char* within the class
TtkString::TtkString(int i)
{

std::stringstream s;
s << i << std::ends;
std::string myString = s.str();
const char* localString = myString.c_str();
int size(strlen(localString));
string = (char *) malloc((size + 1)*sizeof(char)) ;
memset(string,0,size+1);
strncpy(string, localString,size);

}

Seems there is a lot of unnecessary use of temps:

std::stringstream s;
s << i;
string = new char[s.str().length() + 1];
strcpy( string, s.str().c_str() );

The above accomplishes the same thing, in the same way with half the
code. Makes things much easer to understand IMHO.

I have to wonder though, TtkString is far from legacy if it uses
std::string inside itself. Just make it a Adaptor for std::string
instead. I.E.:

class TtkString {
std::string rep;
public:
// member-functions just delegate calls to std::string
// possibly making some modifications along the way
};

reading the code below I see that stringstreams are used (which seems
to me to be a bit heavyweight) and in addition a std::string is
constructed just to get the resulting const char*, subsequently the
malloc is done followed by a memset and a strcpy. It all seems a little
heavy to me (but I stand open to correction...perhaps this is not such
a bad approach altogether).
I was considering using something like this for the body of the same
function....

string = (char*) calloc (1, 33); // 32 bit system assumed.
memset(string,0,33);
itoa(i, string, 10);
however this is not working....Ive obviously messed something up. Can
anybody shed some light?

I don't have itoa, maybe if you could shed some light as to what is not
working about it?

--
There are two things that simply cannot be doubted, logic and perception.
Doubt those, and you no longer*have anyone to discuss your doubts with,
nor any ability to discuss them.

[mlimber]

bo*@blah.com wrote:
[snip]

thanks for any assistance/input.

You may be interested in Alexandrescu's article on building custom
string classes:

http://www.ddj.com/dept/cpp/184403784

The code from that article became flex_string in the Loki library,
which can be found here:

http://sourceforge.net/projects/loki-lib/

Cheers! --M