[boost] Re: any interest in immutable_string/string pooling ?

From: Bohdan (warever_at_[hidden])
Date: 2002-10-03 13:00:11

"Craig Henderson" <cdm.henderson_at_[hidden]> wrote in message
news:anhmca$b7t$1_at_main.gmane.org...
> I'd be interested in this kind of class. I have implemented a similar type
> of class using the boost CRC library to calculate the CRC of the string and
> using that for comparisons. It is far less feature-rich that your
> implementation and does not use any pooling yet - it is still in the early
> development stage for a specific use where I needed to optimise lots of
> string compares.

If it is possible, i'd like to look at your implementation.
Even if it is very draft version. At this point i'd like to
hear more ideas about stuff and its implementation. Definitely
CRC in string is something new for me.

> Given the other items on my "todo" list, it isn't going to get done anytime
> soon, so I'd be happy to replace it with your submission ;-)
>
> Some observations on the first look:
> 1) the begin() and end() members should be const if the return is
> const_iterator, otherwise not. So
>         iterator         begin() const   { return str().begin(); }
>         iterator         end()   const   { return str().end();   }
>         const_iterator   begin()         { return str().begin(); }
>         const_iterator   end()           { return str().end();   }

Sure, just typing mistake, but since iterator==const_iterator (string is
immutable)
it works correctly. Actually all non-const std::string like member are surplus.
Thanks for spotting it, i'll fix it when collect more opinions.

> should be
>         iterator         begin()               { return str().begin(); }
>         iterator         end()                  { return str().end();   }
>         const_iterator   begin() const  { return str().begin(); }
>         const_iterator   end()    const  { return str().end();   }
>
> 2) you have defined operator<() but not operator>()

Yep, bug #2. But AFAIK map & set uses less operator.

>
> 3) the operator<() uses reinterpret_cast<int> to case a pointer to an int.
> a) Shouldn't this be an unsigned type?

i think that it does not matter, but i'll chech it.

> b) Shouldn't this be static_cast<>?

pointer -> int ? Hmm .. AFAIK static_cast<> is a compile time cast
between castable types. I do not thing that pointer -> int conversion
is legal.

> c) Does it need casting at all?

Good point :) actually reinterpret_cast<...> was artifact from my older code,
that uses hash_map::iterator in place of node_type *. Definitely i should fix
it.

>
> 4) the typedef unsigned size_type; is used to return sizes from the
> encapsulated basic_string, so should be
> typedef typename std::basic_string<CharT>::size_type size_type;

*** Agree, but don't be hypercritical it is just draft to show concept.

>
> 5) there are no revere_iterator or rbegin()/rend() methods; const/non-const,
> of course :-).

Most probably i'll delegate whole const std::string iterface, but it'll be
later.

>
> 6) It might be nice to have the symbol table parameters definable by the
> user somehow. The INITIAL_SIZE and GROW_TIMES constants are hard coded and
> not overridable, but the library user may be able to make a more informed
> decision about these values that the library implementor.
>

see:***

> 7) A mute point, but the THRESHOLD member of symbol_table should be static
> as it is a constant.

  Sure, but bcc32 5.6 and VC7  have malicious static const behavior.
  This means that you can not simply write:

         static const float THRESHOLD = 1.5;

  ,but
         static const int THRESHOLD = 1;

   is valid.
   Definitely i can make it static.   Again ***.

> I appreciate that the basic_immutable_string class
> instantiates a static member of this class, but the static-ness should be
> within the class itself, so clarity and reuse. In fact, there is no real
> benefit in having it a member at all, is there?

i'm not sure but possibly, symbol table should deal with some kind of runtime
statistics. As example GROW_TIMES (non-const) can grow during intensive string
creation process, or tuned by user. Just ideas, i don't even have good profiler
for doing such things... Sigh, where is good old Watcom ... with good new std
support.

>
> Regards
> --Craig

Thanks for tete-a-tete review process! :o)

regards,
bohdan.