Private Moose Attributes

By Ovid on April 8, 2016 1:50 PM

Working on a large internationalization (I18N) for one of our clients and I found myself in a curious position where I needed to build an I18N objects from users, companies, and web sites. It's tricky because there are multiple ways the object can be instantiated:

my $i18n = Our::I18N->new( domain  => $domain );
my $i18n = Our::I18N->new( user    => $user );
my $i18n = Our::I18N->new( company => $company );
my $i18n = Our::I18N->new( request => $c->request ); # at the Catalyst layer

Anything consuming the I18N object should be able to do things such as determine country and language, but should not be able see the user, company, or request because they should not be tightly coupled. There are tricks I could do with BUILDARGS to make the above work, but frankly, that's a pain and often a nasty hack. That's when bare Moose attributes and meta hacking come in handy.

Each of those attributes is built as:

has $attribute => (
    is        => 'bare',
    isa       => '...',
    predicate => "_has_$attribute",
);

The is => 'bare' argument tells Catalyst to not make an accessor for this attribute. So how do we get to its value when we need it?

That's when BUILD takes over. When it gets one of those attributes, it looks it up in a dispatch and calls an internal builder method for it. Internally it looks sort of like this:

if ( $self->_has_request ) {
    my $request = $self->_get_private_value('request');
    ...
}

And the code to handle that is:

sub _get_private_value {         
    my ( $self, $attribute ) = @_;   
    my $meta_attribute = $self->meta->get_attribute($attribute);    
    return unless $meta_attribute->has_value($self);  
    return $meta_attribute->get_value($self);  
}

Frankly, it would have been simpler to do this:

has _request => (
    is        => 'ro',
    isa       => 'Catalyst::Request',
    predicate => "_has_request",
);

And in my BUILDARGS, I could rewrite request as _request, but this is such a low-level, core part of the system that I don't want to take the chance that devs casually call private methods for information they should not have (I might even delete these objects as soon as I'm done with them). In fact, I've a talk in mind about that: the more stuff that is built on your software, the more care you should take, though not everyone seems to appreciate why.

7 comments

Tagged as:

moose

7 Comments

blog.urth.org | April 8, 2016 3:30 PM


has _request => (
    is        => 'ro',
    isa       => 'Catalyst::Request',
    init_arg  => 'request',
    predicate => '_has_request',
);

Ovid replied to comment from blog.urth.org | April 8, 2016 7:52 PM

blog.urth.org: that doesn't work because I need to block accessors.

Aristotle | April 8, 2016 8:56 PM

I think this is what you’re after:

has request => (
    is      => 'bare',
    isa     => 'Catalyst::Request',
    trigger => sub {
        my ( $self, $value ) = ( shift, @_ );
        # extract and store country, language, etc here
        ...
    },
);

If you don’t need to refer to that data again elsewhere, you don’t even need to bother with the predicate and certainly not with monkeying with the meta object. (So you can also use this with just Moo.)

Ether | April 9, 2016 12:53 AM

I don't get it. You say "I don't want to take the chance that devs casually call private methods for information they should not have", but you just wrote a method that opens up every attribute to casual inspection.

The BUILDARGS and BUILD subs are handed the full argument list that was passed to the constructor, so you don't even need to store the request object in an attribute at all. Just extract the values from it that you need, and throw away the rest!

around BUILDARGS => sub {
    my ($orig, $class, @args) = @_;
    my $args = $class->$orig(@args);

    # this is thrown away after construction is complete, so extract
    # everything we need now.
    my $request = $args->{request};

    # also set some_field* attributes, based on the request object
    return {
        %$args,
        some_field1 => $request->something,
        some_field2 => $request->something_else,
    };
};

Calling $self->meta from inside a Moose class is a design smell, and there's almost always a better way.

Ovid replied to comment from Aristotle | April 9, 2016 10:01 AM

Aristotle, thank you. That's a much nicer solution.

Ether, your points are well taken, particularly about the meta object. However, BUILDARGS won't do because I don't yet have type safety. The more the code is used, the more careful we should be with it, so using BUILDARGS in that way defeats much of the purpose of using Moose.

Ether | April 12, 2016 6:21 PM

I don't yet have type safety

It looks like the only type checking you're missing is of the request object itself (the regular constructor takes care of the rest), and that's easily remedied:

use Safe::Isa;

my $request = ... # as above
die 'bad request object' unless $request->$_isa('Catalyst::Request');

# continue with extracting required bits from the request

This is much safer than exposing your entire private API to the entire application, and is exactly what BUILDARGS was designed for.

Toby Inkster | April 21, 2016 1:57 PM

Personally, I wouldn't make user, company, etc into attributes at all. Instead I'd write wrappers for the constructor:

sub new_from_user {
    my ($class, $user) = @_;
    return $class->new(
        country  => logic_to_determine_country(),
        language => logic_to_determine_language(),
    );
}

About Ovid

Freelance Perl/Testing/Agile consultant and trainer. See http://www.allaroundtheworld.fr/ for our services. If you have a problem with Perl, we will solve it for you. And don't forget to buy my book! http://www.amazon.com/Beginning-Perl-Curtis-Poe/dp/1118013840/

More info »

Ovid