Stories
Slash Boxes
Comments
NOTE: use Perl; is on undef hiatus. You can read content, but you can't post it. More info will be forthcoming forthcomingly.

All the Perl that's Practical to Extract and Report

use Perl Log In

Log In

[ Create a new account ]

Ovid (2709)

Ovid
  (email not shown publicly)
http://publius-ovidius.livejournal.com/
AOL IM: ovidperl (Add Buddy, Send Message)

Stuff with the Perl Foundation. A couple of patches in the Perl core. A few CPAN modules. That about sums it up.

Journal of Ovid (2709)

Monday April 06, 2009
04:21 AM

Pondering Role Organization

[ #38761 ]

Important Update: Please do not code like I did below. I know that using separate with statements is stupid and I don't understand why I did that in the code below. If you have separate with statements, you won't get method collision detection, so the code below should be written like this:

package PIPs::ResultSetBase::Group;

use Moose;
extends 'PIPs::ResultSetBase::Pips';
with qw(
    PIPs::ResultSet::Role::DoesCreateWithEntity
    PIPs::ResultSet::Role::DoesPromotionSearch
    PIPs::ResultSet::Role::DoesTagSearch
);

__PACKAGE__->meta->make_immutable( inline_constructor => 0 );
no Moose;

1;

Now back to your original post :)

We have some DBIx::Class::ResultSource classes -- Groups, Seasons, Franchises and Collections -- which represent editorial rules around how other things can be grouped. Their corresponding DBIx::Class::ResultSet classes really don't have any behavior that's unique to editorial groups. They're just ways of containing groups of classes. One of them looks like this:

package PIPs::ResultSet::Franchise;

use Moose;
extends 'PIPs::ResultSetBase::Group';

__PACKAGE__->meta->make_immutable( inline_constructor => 0 );
no Moose;

1;

Because we don't inherit directly from our result set class, we've an extra layer that you have to read to fully understand everything. What does that layer look like?

package PIPs::ResultSetBase::Group;

use Moose;
extends 'PIPs::ResultSetBase::Pips';
with 'PIPs::ResultSet::Role::DoesCreateWithEntity';
with 'PIPs::ResultSet::Role::DoesPromotionSearch';
with 'PIPs::ResultSet::Role::DoesTagSearch';

__PACKAGE__->meta->make_immutable( inline_constructor => 0 );
no Moose;

1;

So we're basically doing this:

DBIx::Class::ResultSet
         ^
         |
PIPs::ResultSetBase::Pips
         ^
         |
PIPs::ResultSetBase::Group
         ^
         |
PIPs::ResultSet::Franchise

Ignoring the inheritance in DBIx::Class, we've stacked an extra three levels on top. Why? This has gained us nothing. My goal is eventually to have something like this:

package PIPs::ResultSet::Franchise;

use Moose;
with 'PIPs::Role::DoesResultSet';
with 'PIPs::ResultSet::Role::DoesCreateWithEntity';
with 'PIPs::ResultSet::Role::DoesPromotionSearch';
with 'PIPs::ResultSet::Role::DoesTagSearch';

__PACKAGE__->meta->make_immutable( inline_constructor => 0 );
no Moose;

We can thus see instantly that this class can be treated as a result set, creates an "entity" (long story) and can be searched by promotions and tags. My only concern is that this result set no longer identifies itself as a "group" the way it did when it inherited from the "group" base class. I could create a "DoesGroup" role:

package PIPs::ResultSet::Role::DoesGroup;

use Moose::Role;
with 'PIPs::ResultSet::Role::DoesCreateWithEntity';
with 'PIPs::ResultSet::Role::DoesPromotionSearch';
with 'PIPs::ResultSet::Role::DoesTagSearch';

__PACKAGE__->meta->make_immutable( inline_constructor => 0 );
no Moose;

1;

And the Franchise class would merely do this role and you can call $franchise->meta->does_role('PIPs::ResultSet::Role::DoesGroup'), but that gains me little over inheritance because I no longer have a list of convenient behaviors at the top of the Franchise class.

On the other hand, do I really want that? I often find that code which handles ->can and ->does is a code smell because the object system should handle dispatching and we shouldn't be manually checking our types, but it is useful at times.

I've decided to hold on this refactoring because frankly, I'm unsure what's the right thing to do here. Decisions, decisions ...

The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.
 Full
 Abbreviated
 Hidden
More | Login | Reply
Loading... please wait.