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

Re: Comments on SRFI-13 reference implementation



   Here are some brief (and sometimes cryptic) comments on the SRFI-13
   reference implementation.  Some of these are just style recommendations.

Brad, thanks for the fantastic code review.

I've just about finished the HTMLification of the SRFI, and am going over
your comments on the reference implementation.

   %string-hash:
   By anding with the mask, implementation does no follow specification
   unless bound is a power of 2.

Yeah, good point. I've added this text to the comments above the function:

    ;;; Compute (c + 37 c + 37^2 c + ...) modulo BOUND, with sleaze thrown in
    ;;; to keep the intermediate values small. (We do the calculation with just
    ;;; enough bits to represent BOUND, masking off high bits at each step in
    ;;; calculation. If this screws up any important properties of the hash
    ;;; function I'd like to hear about it. -Olin)

I'll have more to say about this in a following msg.

   string-fold-right:
   should be (kons (string-ref s i) v), not (kons v (string-ref s i))

I must have caught this one, because my source has this right.

   string-unfold-right:
   start with chunk of size 40, not of size 2.

2 was the debugging value, intended to make lots of chunks. It has been
upgraded to 40 for "production;" thanks.

   string-every, string-any:
   I'd prefer to use criterion, not criteria.

D'oh. I've changed this to the proper singular form.

   string-titlecase, string-titlecase!:
   Reverse order in file.

Not important, is it? Two three-line functions defined right next to each
other. 

   string-index, string-index-right, string-skip, string-skip-right, string-count:
   Add internal routines that do the work but no error checking.  Call the
   string arg s for consistency.

There are no module-internal calls to STRING-COUNT.

Adding internal routines for string-index & string-index-right would only
speed up string-titlecase & string-tokenize... but would slow down other
code's use of string-index by the cost of the procedure call from string-index
to the internal %string-index (unless I write separate internal versions
solely for string-titlecase & string-tokenize, which seems like more trouble
than it's worth). So I punted this.

Same for string-skip & string-skip-right.

One day, I'm going to get a reasonable loop macro SRFI-ized, and then
we can cleanly rewrite this code without these losing higher-order iterators.

   string-pad-right, string-pad:
   Need checking for argument n.  Reverse order in files.

I added arg-checking for argument N.

   string-[down|up]case[!], string-fold, string-fold-right, string-concatenate:
   Define % versions that don't do any argument checking.  Use internally.

See above comments.

   string-kmp-partial-search:
   Need to check i

New check ensures that I is an exact int between 0 and the length of
the pattern.

   string-concatenate/shared, string-concatenate, string-join:
   Need to check strings.

There's no cheap way to check an alpha list to ensure that all elements
are alphas. So I don't check these.
    -Olin