Re: [Pspp-cvs] pspp po/en_GB.po po/pspp.pot src/data/ChangeLog...

[John Darrington]

On Tue, Mar 28, 2006 at 09:33:02AM -0800, Ben Pfaff wrote:
     I like the idea of adding more ds_*() functions but I have a few
     comments on the details.
     
     John Darrington <address@hidden> writes:
     
     > +/* Removes leading spaces from ST.
     > +   Returns number of spaces removed. */
     > +int
     > +ds_ltrim_spaces (struct string *st) 
     > +{
     > +  int idx = ds_n_find(st, "\t ");
     > +  if (0 == idx)
     > +    return 0;
     > +
     > +  if (idx < 0 ) 
     > +    {
     > +      int len = ds_length(st);
     > +      ds_clear(st);
     > +      return len;
     > +    }
     
     It might be simpler to just write 
        if (idx < 0)
          idx = ds_length (st);
     and then let ds_replace() handle the rest.

I noticed a strange situation when I was coding/debugging this.
Sometimes the length member did not seem to correspond to the actual
length of the string contained within it, but was somewhat longer. Eg
length = 10, string = "xxxxx\0xxxx" ; This worried me, and I wrote the
code a little more cautiously, even if it wasn't as simple or efficient.
     
     > +  ds_replace(st, &ds_c_str(st)[idx]);
     > +    
     > +  return idx;
     > +}
     > +
     > +
     
     
     > +int
     > +ds_find(const struct string *st, const char cs[])
     
     I've been trying to name ds_*() functions after the
     corresponding str*() functions, where there's one that's
     analogous.  So I'd think that ds_strspn() or ds_span() would be a
     good name for this function.  ds_find() makes me think of
     strstr() or strchr().


OK.  I was sort of following the C++ std::string methods, but I
probably haven't been very consistent there.

     
     It might be better to return ds_length (st) when the string
     consists entirely of characters in cs, as does strspn().  Then
     you wouldn't even need a special case in ds_ltrim_spaces().
     
     > +/* Returns the index of the first character in ST which 
     > +   is NOT an element of the set CS.
     > +   Returns -1 if no such character is found.
     > +*/
     > +int
     > +ds_n_find(const struct string *st, const char cs[])
     
     Similarly, I would name this function something like ds_cspan()
     and change the return value semantics.

So how would you change the semantics?

J'

-- 
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://pgp.mit.edu or any PGP keyserver for public key.

signature.asc
Description: Digital signature