lucy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marvin Humphrey <mar...@rectangular.com>
Subject [lucy-dev] Clownfish::CFC::Hierarchy binding
Date Fri, 06 Jan 2012 01:54:24 GMT
On Thu, Jan 05, 2012 at 07:32:52AM -0000, logie@apache.org wrote:
> URL: http://svn.apache.org/viewvc?rev=1227510&view=rev
> Log:
> First iteration of building binding CFC::Hierarchy to ruby.

\o/

> This work is associated with JIRA: 202.

Tip: if you want to associate your commits with a particular JIRA issue,
include the issue identifier string -- in this case "LUCY-202" -- somewhere in
the commit message.

That way it will show up when you select either "All" or "Subversion Commits"
in a JIRA window.

> --- incubator/lucy/trunk/clownfish/ruby/ext/Clownfish/CFC/Hierarchy.c (added)

Both Lucy and Clownfish::CFC are large collections of classes.  In the Perl
bindings for both, all C code is loaded via a single extension.  That
simplifies client code, because a single "use" directive loads everything.

    # loads Clownfish::CFC::Hierarchy, Clownfish::CFC::Type, etc.
    use Clownfish::CFC;

I think it would be in our interest to do the same for the Ruby bindings, so
that the following directive loads everything related to Clownfish::CFC:

    require "clownfish/cfc"

If I understand correctly, that implies merging all code from
clownfish/ruby/ext/Clownfish/CFC/Hierarchy.c into
clownfish/ruby/ext/Clownfish/CFC.c, and then making sure that invoking
Init_CFC() initializes *everything* under Clownfish::CFC.

> +    rb_mCFC     = rb_define_module("CFC");
> +    rb_cCFC_H   = rb_define_class_under(rb_mCFC, "Hierarchy", rb_cObject);

The class name is technically "Clownfish::CFC::Hierarchy" (as embedded in the
CFCHIERARCHY_META object), so we need one more module.  I think we should also
follow the pickaxe book's convention and use "mFoo" rather than "rb_mFoo" --
staying out of the "rb_" namespace that Ruby has staked out for itself.

    mClownfish = rb_define_module("Clownfish");
    mCFC       = rb_define_module_under(mClownfish, "CFC");
    cHierarchy = rb_define_class_under(mCFC, "Hierarchy", rb_cObject);

FWIW, I explored this technique in a thread on SDRuby this morning to make sure
that it was both safe and correct to allow for rb_define_module() to be called
more than once:

    http://groups.google.com/group/sdruby/browse_thread/thread/5933c140eae04da2

> +static VALUE build(VALUE self) {
> +    CFCHierarchy *rb_oCFC_H; 
> +
> +    Data_Get_Struct(self, CFCHierarchy, rb_oCFC_H);
> +    CFCHierarchy_build(rb_oCFC_H);
> +
> +    return self;
> +}

Perhaps we might consider an alternative naming convention for
"self/rb_oCFC_H"?  I understand what "rb_oCFC_H" means, but it's visually
challenging to parse, and it's only an automatic variable, so it doesn't need
the strong prefixing which would allow it to be placed within a larger
namespace.

In the Perl bindings we're using "self_sv" for the SV* perl wrapper, and
"self" for the Lucy object.  How about "self_rb/self" instead?  We're going to
be using these names *a lot* (including in generated binding code) so we
should try hard to get things right.

Marvin Humphrey


Mime
View raw message