[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index][Thread Index][Top&Search][Original]

Re: Class::Fields, a base/fields.pm rewrite



Sorry about the delay in getting back to this.  Life intervenes.

The impression I got from talking to a few people about this whole
thing is that I was really not clear.  Not surprising.  Let me try to
clear up a few basic things without tripping over my own feet.

1)  This only has a direct effect on pseudohashes (although it is useful
for objects built out of hashes)

2)  I'm not proposing a new OO system.  Just a few minor builds on
what's already there (although what's there is experimental).


On Sun, Jan 02, 2000 at 11:28:02AM -0800, Gurusamy Sarathy wrote:
> On Wed, 29 Dec 1999 15:47:33 EST, Michael G Schwern wrote:
> >I found myself having to do direct manipulation of %FIELDS and
> >%fields::attr to get anything done.
> 
> Can you describe the inadequecies you ran into in more detail?
> (I want to understand the issues better before we make any changes.)


The major one is this: fields.pm is a write-only interface.  I can put
new entries into %FIELDS but I have no simple way of examining them
once they're there.

Say I declare a class using fields.pm:

        package Foo;
        use fields qw(this that _nsakey);

Now say I want to find out what the public data members of this class
are.  Here's what you currently have to do (YANETUT applies):

        @public_fields = ();

        # Check to see if %Foo::FIELDS exists.  You can't just say
        # if( %Foo::FIELDS ) because that would autovivify it and
        # that's ungood because it causes field inheritance problems.
        if( $fglob = $Foo::{FIELDS} and *$fglob{HASH} ) {
            # Now we "simply" iterate through the keys in %Foo::FIELDS and
            # check each against its relevent entry in @{$fields::attr{Foo}}
            @public_fields = 
              grep { 
                    ($fields::attr{Foo}[$Foo::FIELDS{$_} - 1] & 
                       fields::_PUBLIC) == fields::_PUBLIC 
                   } 
                keys %Foo::FIELDS;
        }

Was this trip really necessary?  Not only is it a mess and prone to
subtle bugs (in fact, I just noticed that my own implementation of
Class::Fields has an autovivification of %FIELDS bug) but worse it
exposes the internal workings of field inheritance.  Workings which
will be very prone to change.

I propose Class::Fields as a wrapper around all this.  With it, one
can simply say:

    use Class::Fields;
    @public_fields = show_fields('Foo', 'Public');

show_fields() simply encapsulates the above mess and shields the user
from any future internal changes.  Its also a hell of alot more
obvious what's going on.  Class::Fields also provides a few other
functions to do examination of individual fields and a debugging
function to dump the fields table completely.

I personally needed this for Class::Accessor.  It autogenerates
accessor methods for public data members.  Its autoloader does
something like this (this is distilled for clarity):

      use Class::Fields;
      sub AUTOLOAD {
          my($self) = $_[0];
          my($class) = ref $self || $self;
          my($method) = $AUTOLOAD =~ /::([^:]+)$/;

          if( is_public($class, $method) ) {
              # make the accessor and goto it.
          }
          else {
               die "'$method' is not a public data member of '$class'";
          }
      }

I can also see it being useful for debuggers.


So that's the major issues: Unnecesary exposure of the internals and a
gross interface for field examination.

There are a few minor ones.  First of which is the implicit
association of fields.pm with pseudo-hashes.  I would like to
disassociate them.  I've found it useful to declare my fields even
when using a normal hash-as-object (you'll note that nothing I've done
so far relies on pseudohashes).

I gain two things from this.  First, I can explicitly declare my
fields, even if its on a voluntary basis.  This helps with
readability, organization and maintainance and its more consistant
than a bunch of comments.

Second, I can add bits of BD/SM style OO where I feel its necessary.
For example, a DESTROY routine which checks for undeclared data members:

    package Bar;
    use base qw(Class::Fields);
    use fields qw(foo bar _this);

    sub new { bless {} }

    sub DESTROY {
        my($self) = @_;
        my($class) = ref $self;

        my %fields = map { ($_,1) } $self->show_fields;
        foreach my $key ( keys %$self ) {
            warn "Strange key '$key' found in object '$self' ".
                 "of class '$class'" unless
                exists $fields{$key};
        }
    }

    package main;
    $bar = Bar->new;
    $bar->{fop} = "Ben Folds Five";

    # A warning will be thrown about 'fop' when $bar is destroyed.

This gives me -some- object protection.  At least I can catch
violations and typos.  Voluntary discipline, added as I choose.  To
me, this keeps the polite flavor of Perl's OO model while allowing
people to introduce a bunker mentality if they want.

Point is, fields.pm doesn't have to be just for pseudo-hashes anymore.
Class::Fields moves towards this goal by providing documentation which
doesn't assume you're only using this for pseudohashes (though I need
to work on the fields.pm and base.pm docs) and also providing an
interface which deals at a high level of abstraction.

Pseudo-hashes can disappear and Class::Fields will still be useful.
An entirely new OO data type can be introduced and Class::Fields's
interface will still be relevent (with necessary guts surgery, of
course).


Another issue which ties in with the one above is the name itself.
Which is clearer?

    use fields qw(this that _foo _bar);

or

    use public  qw(this that);
    use private qw(_foo _bar);

IMHO the latter is.  "use fields" does not imply any association with
data members or even that this has anything to do with object oriented
techniques.  "use public" and "use private" will be recognized by most
programers familiar with OOP and their purpose should be fairly clear.

Class::Fields provides public.pm and private.pm as a simple renaming
of fields.pm.  The code is almost exactly the same and in the end they
do the same thing.  (I'm pondering calling it public_fields.pm and
private_fields.pm... but I think that waters down the meaning.)

(Also, let me state for the record again that this is NOT an attempt
to impose a new OO system onto Perl, just a bit of sugar.)


Finally, the internals of base.pm and fields.pm are really scary.  It
took me alot of poking about until I fully understood what the hell
was going on well enough to attempt any changes.

For example, this is the import routine from fields.pm:

sub import {
    my $class = shift;
    my $package = caller(0);
    my $fields = \%{"$package\::FIELDS"};
    my $fattr = ($attr{$package} ||= []);

    foreach my $f (@_) {
        if (my $fno = $fields->{$f}) {
            require Carp;
            if ($fattr->[$fno-1] & _INHERITED) {
                Carp::carp("Hides field '$f' in base class") if $^W;
            } else {
                Carp::croak("Field name '$f' already in use");
            }
        }
        $fields->{$f} = @$fattr + 1;
        push(@$fattr, ($f =~ /^_/) ? _PRIVATE : _PUBLIC);
    }
}

Not pleasent to look at, unclear what its doing and requires intimate
understanding of what %attr is all about.

Here's my proposed replacement:

use Class::Fields::Fuxor;       # for add_fields()
use Class::Fields::Attribs;     # for PUBLIC and PRIVATE
sub import {
    # Dump the class.
    shift;
        
    my $package = caller(0);
        
    return SUCCESS unless @_;

    add_fields($package, PRIVATE, grep {/^_/} @_);
    add_fields($package, PUBLIC,  grep {!/^_/} @_);
}

Simple, straight forward and easy to understand.  It also guts
fields.pm and turns it into what it should be, nothing but an
interface.  Currently fields.pm and base.pm are very incestuous with
each other, calling each other's routines for no good reason.


Finally, there is the issue of expandability.  As mentioned, fields.pm
and base.pm are currently tightly bound and written in such a way that
prevents the relatively easy authoring of alternative field
interfaces.  Class::Fields::Fuxor servers to hold all the
functionality necessary to fuck around with the guts of the fields
interface.  By making introducing this module, and making fields.pm
and base.pm rely upon it instead of using their own functions, you
allow other authors to write their own modules to play with data
fields while being reasonably assured that it will survive future
changes to the fields system.


I think that about covers it.  Hiding internals.  Disassociation from
pseudohashes.  Improving readability.  Improving the existint
interface.

There are other major issues about the existing system (multiple data
member inheritance being the big one), but Class::Fields does not
address them so I won't go into it here (I will go into it in another
post, I might have a solution.)


Ugg, looking back at this monster I probably just confused things again.

> Did you know fields.pm was originally called Class/Fields.pm?
> I find that rather funny. :-)

I had no idea.  I'll have to switch to heavy-duty tinfoil for my
hats.  Get out of my mind.

-- 

Michael G Schwern                                           schwern@pobox.com
                    http://www.pobox.com/~schwern
     /(?:(?:(1)[.-]?)?\(?(\d{3})\)?[.-]?)?(\d{3})[.-]?(\d{4})(x\d+)?/i


References to:
Gurusamy Sarathy <gsar@ActiveState.com>

[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index][Thread Index][Top&Search][Original]