byterock

If you have been following along you may remember this post, in which I introduced what my API was going to look like. Up till now I had only a few of the basic parts of that list in the in Accessor.pm jsut for quick testing. Now before I add in these new elements into my code I think it is time to review my Accessor.pm and see how I can clean it up before I add the others.

So I started with the 'View' and 'Element' classes, bundled in Accessor.pm and I see that I still have a 'retrieve' sub in each. A few posts ago I gave up the idea of trying to enforce the DAD writers to create these subs so there no reason to have them there at all. So out they go.

Now that leaves up with two identical classes save for their name, looks like the beginning of an anti-pattern?? Both of these are used in the Accessor.pm in 'View' and 'Elements' attributes, side note here, I am going to go with upper camel case for all Accessor attributes, so really there is no need to have separate classes for both them now that I got rid of the 'retrieve' subs.

So I think I will get rid of the View class altogether and that leaves the Element class but it that the proper name for it?? I still have the View and Elements attributes but the users of Accessor do not really have to know what 'class' goes in there as long as it works. However, it is always a good idea to have your accessors match up the data types that they return. Always a pain when you see an attribute like 'field_count' and it returns a string like 'There are 7 items in the field'. So I will have to make a point in my documentation to say something like; A 'View' is a single 'Element' object used to describe the data container you are interested in, or alike.

Now in the long run one of the key objectives of mine is to eliminate the need to do something this;

my $address = Database::Accessor->new({

        view     => Database::Accessor::View->new({name  => 'person',

        alias    => 'me'},

        elements =>[Database::Accessor::Element->new( { name => 'street', } ),

…

or like in my tests where I do a bunch of these

my $view = Database::Accessor::View->new(

    {

        name  => 'person',

        alias => 'me'

    }

my $street = Database::Accessor::Element->new( { name => 'street', } );

…

then create the address like this

my $address = Database::Accessor->new(

    { view     => $view,

      elements => [$street,

...

What I would much rather have is something like this

Database::Accessor->new({View   =>{name =>'person',

                                   alias=>'me'},

                        Elements=>[{name=>'street'},

                                   {name=>'city'},

                                   {name=>'country'}]});

Much much cleaner without all those class names and news about and it less error prone as there are simply fewer things to type, a good thing for someone with my typing skills.

Moose has a feature I really love called Coercion and lets up set up the above with only just a little trouble but in the end well worth it as it makes your API very tight and consistent across attributes and you get the bonus feature that validation on your parameters can be simplified and standard in an API.

Now some out there might ask why don't you simplify it even more like this

Database::Accessor->new({View=>{person => 'me'},

                         Elements=>['street',

                                    'city',

                                    'country']);

Well the problem with that minimal approach is you start to make assumptions in this case the alias of 'person' is 'me' but only careful reading of the API documentation would tell you that, if it is even witten up. As well we could have this case of a View that did not have an alias and some fields that had separate views

Database::Accessor->new({View=>'person'},

                         Elements=>['street',

                                    'city',

                                    'country',

                                    {name=>'country_id',

                                     view=>'countries',}]);

Suddenly you have a very inconstant API, sometimes taking a hash other times taking just a scalar and somethime both at the same time. Now Moose Coercion can handle all of the above but all you are going to do is confuse and piss off your users as well you will have to include Coercion code for each and ervery case you want to include.

Much better to take the first approach a hash for each and allow a little leniency by not requiring every field and making sure that the it is plainly explained in the documentation.

So now just to code that up and I am off to the races.