NEILB's Blog

I'm working on a Perl wrapper for the BambooHR API, and using this as a learning experiment: deliberately trying different approaches and iterating on the design. One aspect of the design is the bit that actually makes the HTTP requests. This started off as part of the main class, but here I describe different refactorings I tried, learning a bit more about Moo as I went.

This post is to record my evolving thinking and experience, for myself as much as anything. I've written a lot of OO Perl classes, but many were in the old, pre-Moose / Moo days, and recently I've just done simple things with Moo thus far.

Often I seem to refactor 'late', focusing on getting the job done, but for this project I'm intentionally refactoring early, to see whether I feel like I get to a good / better solution earlier.

First cut

I'm using Moo, and the main class is WebService::BambooHR. To talk to the API you need an API key and a company name.

$bamboo = WebService::BambooHR->new(
                api_key => '...',
                company => '...');
@employees = $bamboo->employees();

I had two private methods for making requests:

has 'api_key' => (is => 'ro');
has 'company' => (is => 'ro');

sub _get  { ... };
sub _post { ... };

And then public methods do something like:

sub employees {
    my $self     = shift;
    my $response = $self->_get('employees/directory');

I decided to refactor this to pull out the _get() and _post() methods. They're only two methods at the moment, but they do a number of things and I was mentally thinking of the toplevel class as two separate chunks of code: the public methods and the private stuff.

UserAgent class - first try

The toplevel class needs something to make the requests and handle the low-level details, so it felt like a 'has a' relationship, not an 'is a'. So I created a class:

package WebService::BambooHR::UserAgent;
use Moo;

has 'api_key' => (is => 'ro');
has 'company' => (is => 'ro');

sub _get  { ... };
sub _post { ... };

The toplevel class still needs to take an API key and company name, so I had:

has 'api_key' => (is => 'ro');
has 'company' => (is => 'ro');
has 'ua'      => (is => 'rw');

sub BUILD {
    if (!defined($self->ua)) {
        my $ua = WebService::BambooHR::UserAgent->new(
                    api_key => $self->api_key,
                    company => $self->company);
        $self->ua($ua);
    }
}

That didn't feel clean though, so I went on IRC and described what I was trying to do.

Lazy builder

Graham Knop (haarg) pointed out the lazy builder mechanism, which was meant for this. So my code became:

has 'ua' => (is => 'lazy');

sub _ua_builder {
    return WebService::BambooHR::UserAgent->new(
                api_key => $self->api_key,
                company => $self->company);
           );
}

That's better, but the duplication of the attributes still felt sub-optimal.

Delegation?

Joel Bernstein (joel) questioned whether a separate class was really called for, and this perhaps should stay part of the class. During this discussion, Jarrod Funnell (Timbus) suggested that delegation might be what I was looking for. He pointed me at the Moose doc on delegation.

I knocked up a play class based on the example in the doc. This seemed promising, so I wrote this in the toplevel class:

has 'ua' => (
    is      => 'lazy',
    handles => ['api_key', 'company'],
);

But I couldn't get this to work. I applied the 15 minute rule and banged my head against it for the requisite time before asking again on IRC. Dagfinn Ilmari (ilmari) pointed out that the handles feature is for delegating methods not attributes.

The documentation's abstract says "Attribute delegation", but actually the rest of the doc makes it clear that it's for delegation of methods not attributes.

Attributes just seem like a special kind of method to me at the moment, but obviously that's not the case. Oh well.

UserAgent role

Early on in the IRC discussion Joel had suggested that I could just use a role. As I said above, it felt to me like the API class should have a user agent rather than be one, which is why I initially avoided going for a role.

But at this point I decided that making it a role would remove the duplication of attributes, and make the code cleaner. The framework of the user agent then became:

package WebService::BambooHR::UserAgent;
use Moo::Role;

has 'api_key' => (is => 'ro');
has 'company' => (is => 'ro');

sub _get  { ... };
sub _post { ... };

And the top class:

package WebService::BambooHR;

use Moo;
with 'WebService::BambooHR::UserAgent';

sub employees { ...  }

That'll do for now. I've got the decomposition I was after, and this is the cleanest of all the different things I tried, even if it still doesn't feel quite right. I'll come back in six months and see if I've changed my mind.

Finally

Thanks to joel, haarg, Timbus and ilmari for their help and patience. As ever, asking questions (in this case on #moose) was a positive experience.