homegrown string class optimisation

Discussion in 'C++' started by bob@blah.com, Oct 19, 2006.

  1. Guest

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

  2. Greg Guest

    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
     
    Greg, Oct 19, 2006
    #2
    1. Advertising

  3. Daniel T. Guest

    "" <> wrote:

    > 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.
     
    Daniel T., Oct 19, 2006
    #3
  4. mlimber Guest

    mlimber, Oct 19, 2006
    #4
    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. Fredrik Ramsberg

    Optimisation of regexps in Perl?

    Fredrik Ramsberg, Oct 14, 2003, in forum: Perl
    Replies:
    2
    Views:
    475
    Fredrik Ramsberg
    Oct 15, 2003
  2. Roedy Green

    boolean loop optimisation

    Roedy Green, Sep 11, 2003, in forum: Java
    Replies:
    8
    Views:
    2,828
    Chris Uppal
    Sep 12, 2003
  3. sorry.no.email@post_NG.com

    Search Engine Optimisation

    sorry.no.email@post_NG.com, May 8, 2006, in forum: HTML
    Replies:
    0
    Views:
    352
    sorry.no.email@post_NG.com
    May 8, 2006
  4. Oliver Batchelor
    Replies:
    1
    Views:
    373
    Frank Schmitt
    Jul 22, 2003
  5. Omar
    Replies:
    0
    Views:
    340
Loading...

Share This Page