+0 I think it's the right move to update methods to return LIST context since that just feels intuitive. However, I'm apathetic in regard to the methods returning both SCALAR and LIST context, since this feel ambiguous to me since older methods still return in SCALAR context. However, I feel this is the right direction in most regards.
Thanks, Logan On Sun, Mar 25, 2012 at 11:01 AM, Marvin Humphrey <[email protected]>wrote: > Greets, > > One idiosyncracy of Lucy's Perl interface is the consistent avoidance of > list > context. Ordinarily, we would want to embrace such a host language idiom, > but for a couple reasons, we have not. > > The biggest problem is that it is not possible to provide a bulletproof > type mapping between a Clownfish method which returns a VArray* and list > context in Perl. A method which returns a VArray* might return any of the > following: > > 1. NULL > 2. A VArray with no elements. > 3. A VArray with some elements. > > List context conflates NULL with an empty array -- but those are not always > the same, since NULL might signify an uninitialized state. If we map to a > Perl arrayref, though, we can properly represent all possible return > states, > solving the problem. > > Historically, at one point our only choices were an autogenerated binding > returning an arrayref vs. hand-rolled XS returning a list, and so some > interface design sacrifices were made in order to get rid of error-prone > hand-rolled XS code. For instance, Schema#all_fields used to return a > list, > but if I recall correctly it was changed to return an arrayref when Schema > was > ported to C so that it wouldn't be necessary to write a hand-rolled XS > binding > and so that Lucy would present a consistent interface across the whole > library > when returning arrays. > > Looking forward, it might be nice to allow for our bind_method() routine to > support list context, allowing methods such as Schema#all_fields to return > lists. > > Current usage: > > for my $field ( @{ $schema->all_fields } ) { > ... > } > > Possible binding spec change: > > my $binding = Clownfish::CFC::Binding::Perl::Class->new( > parcel => "Lucy", > class_name => "Lucy::Plan::Schema", > ); > $binding->bind_method( > alias => 'all_fields', > method => 'All_Fields' > list_context => 1, > ); > > New usage: > > for my $field ( $schema->all_fields ) { > ... > } > > The reason I bring this up now is because I saw that Nick had bound a > couple > methods within CFC to return in list context. I was initially concerned by > the inconsistency, but times have changed since the initial port of Lucy > to C, > and after thinking things over, avoiding list context no longer seems > mandatory. > > FWIW, there are a number of other methods within CFC that arguably ought to > return a list. However, I was kind of looking forward to shrinking CFC.xs > and > lightening our maintenance burden once we port CFC's test files to C, so > making those changes when CFC isn't public in any case seems like a > low-yield > endeavor. > > Marvin Humphrey > > On Thu, Mar 22, 2012 at 11:46 AM, <[email protected]> wrote: > > > +void > > +get_source_dirs(self) > > + CFCHierarchy *self; > > +PPCODE: > > + size_t n = CFCHierarchy_get_num_source_dirs(self); > > + size_t i; > > + EXTEND(SP, n); > > + for (i = 0; i < n; ++i) { > > + const char *value = CFCHierarchy_get_source_dir(self, i); > > + PUSHs(sv_2mortal(newSVpv(value, strlen(value)))); > > + } > > + >
