lucy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Wellnhofer <wellnho...@aevum.de>
Subject Re: [lucy-dev] Clownfish String API
Date Sun, 18 Oct 2015 12:01:44 GMT
On 15/10/2015 21:21, Marvin Humphrey wrote:
> On Wed, Oct 7, 2015 at 4:10 AM, Nick Wellnhofer <wellnhofer@aevum.de> wrote:
>> - Make stack strings public?
>
> I'd say wait for now.  They're truly an expert API -- it's too easy to blow
> the C stack by using them in a loop, etc.

OK.

>> - Make `Find` return a size_t.
>>    Requires special value for "not found".
>
> Hmm, that's a toughie.  If the sentinel is SIZE_MAX, that might not fit in all
> host numeric types.

Good point. The problem is not so much the byte size of the return type but 
the fact that a host language might not support unsigned integers. Maybe we 
should limit string sizes to SSIZE_MAX and make `Find` return an ssize_t. But 
this requires to emulate the ssize_t type on non-POSIX platforms.

> In addition, I'd suggest:
>
> *   Remove `Str_less_than`, replacing it in PolyLexicon with
>      `Str_Compare_To(a, b) < 0`.
> *   Remove `Str_compare` from the public API.

+1

> *   Remove `new_from_char`.

-0.5

> *   Remove `BaseX_To_I64`, optionally adding its functionality to
>      StringHelper.

+0

> *   Rename the argument to `Ends_With` from `postfix` to `suffix`.  Same with
>      `Ends_With_Utf8`.
> *   The return values for `Trim`, `Trim_Top`, and `Trim_Tail` should be
>      `incremented`.

+1

>> StringIterator
>>
>> - Rename StrIter_substring to Str_new_from_iter?
>
> Hmm.  I see where you're going with that, but new_from_iter doesn't
> communicate that there must be a single backing String for the two
> StringIterator arguments.  Maybe `StrIter_crop`?

I'm also OK with either StrIter_substring ot StrIter_crop. I only thought it 
might be clearer to make this function a constructor of the String class.

>> - Make `Starts_With`, `Ends_With` skip over strings?
>>    I think this better matches current usage patterns.
>
> Somehow I don't understand what you mean.

I mean that `Starts_With` should be named something like `Advance_If_Match` 
and make the iterator move past the prefix if it matches. Currently, most code 
that calls StrIter_Start_With looks like

     if (StrIter_Starts_With(iter, prefix)) {
         StrIter_Advance(iter, prefix_len);
         ...
     }

This could be replaced with a simpler and more efficient

     if (StrIter_Advance_If_Match(iter, prefix)) {
         ...
     }

>> - Find better names for `Skip_{Next|Prev}_Whitespace`?
>
> `Advance_Whitespace` and `Recede_Whitespace`?

As a non-native speaker, I'm not really sure.

> While preparing this reply, I also created the patch below with some doc
> tweaks.

Looks good. I will start off with your patch.

Nick



Mime
View raw message