There is no good or bad, just fun and not fun

Previous Entry Add to Memories Share Next Entry
Why do I always find solving meta problems more fun than real problems.
clone
bobtfish
I have a feeling that I fail at ironman. Meh, oh well - I'll keep blogging anyway, of course - I'm not about to stop ranting about shit for at least a while yet, sorry..

In sympathy for mst, I might have to also make my hair pink (which is the colour I've most often heard mentioned in reference to that), as I fail miserably, and before him...

Oh, no, wait - my hair is pink, DOUBLE FAIL.

Anyway, onto what I was going to rant about.. In the last episode, I was working on Role support for MooseX::MethodAttributes.

This is now released, works, has a much nicer way of asking for it, and has been used in anger by the crazy kids over at MusicBrainz. Go upgrade to 0.12 now. :)

I've still not got round to writing up a really complex example, and/as I think that's actually going to turn into something on github you can play with, and a series of posts...

But for now, here is another trivial use case, which gets messy when you think about it on a larger scale.

Say you work for a company, and you have multiple (different) Catalyst apps, but you want to reuse things and have them 'work the same' between your different apps for consistency.

So in each site, you're going to have a sitewide wrapper template, and it's going to end up like this:

wrapper = wrapper || 'site/layouts/full.tt2'

which is okish. This will use the wrapper specified in your code, or the default wrapper..

But how about if you want to skin your site, or do a/b testing with two different site layouts, or..

Having that chunk of string buried in your template is kinda rubbish now, huh?

So you write this:

wrapper = wrapper || default_wraper

And then you write this (some context shown):

package MyCompany::MyApp::Controller::Root;
use Moose;

BEGIN { extends 'Catalyst::Controller' }

sub root : Chained('/') PathPart('') CaptureArgs(0) {
    my ($self, $c) = @_;
    # N.B. This is the only line added, rest of code was already there ;)
    $c->stash->{default_wrapper} = $c->view('HTML')->default_wrapper;
    # More stuff to do for _every_ page here.
}
...
package MyCompany::MyApp::View::TT;
use Moose;

extends 'Catalyst::View::TT';

has default_wrapper => ( is => 'ro', required => 1, isa => 'Str' );

...
<MyApp::View::TT>
   default_wrapper site/layouts/full.tt2
</MyApp::View::TT>


And suddenly the configuration value is in config. Woohoo, you're now correctly design pattern compliant, as you're not storing app config data in the templates :)

Now, back to the point - your company has a lot of apps.. They're all going to do this. So what, it's one line of code. WRONG

DON'T REPEAT YOURSELF

DON'T REPEAT YOURSELF. (Look, I fail! Oh, we covered that once..)

This won't be the only commonality, remember, this is a trivial example so lets think up a way of doing this generically, so we don't copy code around. The traditional way to abstract that would be to provide a 'MyCompany' base class with the default 'sub root'.

Ok, that's cool - except you suddenly realise that you have 6 features like this, but if you add all 6 to all of your apps, it breaks things.

FAIL

Whatcha gonna do?

package MyCompany::ControllerBase::Root;
use Moose;

BEGIN { extends 'MyCompany::ControllerBase'; }

sub root : Chained('/') PathPart('') CaptureArgs(0) {
    my ($self, $c) = @_;
    if ($c->config->{features}{has_feature_1}) {
        # Do stuff for feature one
    }
    if ($c->config->{features}{has_feature_2}) {
        # Do stuff for feature two
    }
    # Repeat, 4 more times
}

Sorry, you fail at OO. Next.

package MyCompany::ControllerBase::Root::FeatureOne;
use Moose;

sub root : Chained('/') PathPart('') CaptureArgs(0) {
    my ($self, $c) = (shift, shift);
    $c->stash->{default_wrapper} = $c->view('HTML')->default_wrapper;
    $self->next::method($c, @_);
}
... Repeat, 5 more times

I laugh the special laugh I save for people who think that having 6 base classes is a good idea, and we move on now.

package MyCompany::CatalystX::FeatureOne;
# Code may contain up to one lie, see if you can spot it before you read the explanation.
use strict;
use warnings;

sub import {
    my $caller = caller();
    no strict 'refs';
    &{$caller."::_add_feature"}(sub {
        my ($self, $c) = @_;
        $c->stash->{default_wrapper} = $c->view('HTML')->default_wrapper;
    });
}

package MyCompany::ControllerBase::Root;
use Moose;

BEGIN { extends 'MyCompany::ControllerBase'; }

{
    my @features;
    sub _add_feature { push @features, shift }
    sub _run_features { $_->(@_) for @features }
}

sub root : Chained('/') PathPart('') CaptureArgs(0) {
    my ($self, $c) = @_;
    _run_features($self, $c);
}

package MyCompany::MyApp::Controller::Root;
use Moose;

BEGIN { extends 'MyCompany::ControllerBase::Root'; }

Wow. Note the crack fueled non-generic implementation of part of method modifiers and part of roles. Note also, this doesn't actually work in your base class as shown because it relies on the caller. Fairly easy to fix with more code..

The fact that it doesn't work yet and that you can't use namespace::clean (without an 'avoid the mad shit' incantation) just adds to the charm.. ->can('has'). No, really I actually hate this least so far.

So anyway, after wallowing in filth. Let me present:

package MyCompany::CatalystX::FeatureOne;
use Moose::Role;
use namespace::autoclean;

requires 'root';

after 'root' => sub {
    my ($self, $c) = @_;
    $c->stash->{default_wrapper} = $c->view('HTML')->default_wrapper;
};

package MyCompany::CatalystX::DefaultRoot;
use Moose::Role -traits => 'MethodAttributes';

# Yes, I need no body, all the functionality is in modifiers
sub root : Chained('/') PathPart('') CaptureArgs('') {} 

package MyCompany::MyApp::Controller::Root;
use Moose;

BEGIN { extends 'MyCompany::MyApp::ControllerBase' }

with map { "MyCompany::CatalystX::ControllerRole::$_" } qw/
    DefaultRoot
    Feature1
    Feature2
    Feature3
/;

Tada, much more elegant.

And you can split it how you like - have the features which really are common for all apps in your MyCompany base class, sure. Have the action in the base class and just apply modifiers to it, sure.

Whatever works for you, helps you write cleaner code and allows you to avoid repeating yourself - it's all good with me, I'm just pointing out how taking advantage of MooseX::MethodAttributes to do something else that can be done in 1 line might actually be a good option for maintainable and reusable application code.

Whoda thunked that?

you need to get more stoned my good man thn all will be revealed:P

You are viewing bobtfish