On 24/01/2011, at 1:58 AM, Joseph Wright wrote: > I'm happy with the idea in principal, but not the names or the > implementation :-) You had me worried there for a second, but your implementation is just an optimisation of mine -- phew! At least we're on the same wavelength there :) Thanks for the feedback; exactly what I was hoping to hear. > On the names, we currently have \tl_elt_count:N, so should really follow > the model and have \clist_elt_count:N. On the other hand, I notice that > there are various other uses of 'element' in other functions. So perhaps > we'd be better having \tl_element_count:N and \clist_element_count:N, > and not using 'elt' at all. (We also need :c and :n variants.) If we were going to change the _elt_count one, wouldn't _length be the most sensible for all of them? > For \clist_nth:N, I'd prefer '\clist_element:N'. We don't want 'get' > here as that is used in non-expandable assignments (see \prop_get:NnN, > etc.). 'nth' is really awkward, I think. Sure, I agree here. > On the implementation, there are some things to think about. You've gone > for an implementation indexed from 1, which gives an error with 'out of > range' cases. In other places we've indexed from 0 (as TeX does for > \ifcase), and we need to cover negative numbers. I'd prefer to avoid two > loops, and also think that 'out-of-range' values should simply result in > an empty value, not an error. Oh yes -- bailing at the end of the list rather than explicitly checking sizes is a fine idea. For negative indices, I think they should actually be used to access elements from the end. > A couple of notes. I've gone for \int_compare:nNn for performance > reasons over \int_compare:n, Sure. Efficiency comes second after working implementation :) > and I'd probably use the expansion of > \int_eval:n in the 'count' functions, so that it only requires two > expansions to get to the result rather than three. You mean avoiding the \int_eval:n inside the 'f' expansion so you get "n-1-1-1-1-1-1-1-1==0" as the test goes along? I doubt it matters too much in terms of efficiency, and this makes the code more readable, so I've changed it to this approach. * * * Based on all this, I've updated the SVN repository with a new version of the code (but haven't done the seq versions, yet). Cheers, -- Will