pspp-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Ben Pfaff
Subject: Re: [Pspp-cvs] pspp po/en_GB.po po/pspp.pot src/data/ChangeLog...
Date: Tue, 28 Mar 2006 22:52:43 -0800
User-agent: Gnus/5.110004 (No Gnus v0.4) Emacs/21.4 (gnu/linux)

[switching discussion to pspp-dev]

John Darrington <address@hidden> writes:

> On Tue, Mar 28, 2006 at 09:33:02AM -0800, Ben Pfaff wrote:
>      > +  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.

Oops.  You caught me there.

I think the correct solution is to fix ds_replace().  It should
probably take a length argument.  Perhaps it should be renamed
ds_copy(), for consistency.

>      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.

I'm not very familiar with std::string, so that's probably why I
didn't notice that.

>      > +/* 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?

Same way as ds_find(): in the case where every character is in CS,
return the length of the string.

None of this is a big deal, but I do like consistency.  I'm happy
to roll these changes into a change set I'm working on, if that's
okay with you.  I'll of course fix up call sites as needed.
-- 
"I consider that the golden rule requires that if I like a program
 I must share it with other people who like it."
--Richard Stallman




reply via email to

[Prev in Thread] Current Thread [Next in Thread]