Search code examples
perlmoose

Rewriting object attributes - best way to do it using Moose?


Let's see whether the SO question entry robot prediction, apparently issued based on just the question title, will come true:

The question you're asking appears subjective and is likely to be closed.

Using Perl/Moose, I'd like to bridge a mismatch between two ways merchant articles are represented. Let an article have name, quantity and price. The first way this is represented is with quantity set to any numeric value, including decimal values, so you can have 3.5 meters of rope or cable. The second one, which I have to interface with, is, alas, inflexible, and requires quantity to be an integer. Hence I have to rewrite my object to set quantity to 1 and include the actual quantity in the name. (Yes, this is a hack, but I wanted to keep the example simple.)

So the story here is that one property's value affects other properties' values.

Here's working code:

#!perl
package Article;
use Moose;

has name        => is => 'rw', isa => 'Str', required => 1;
has quantity    => is => 'rw', isa => 'Num', required => 1;
has price       => is => 'rw', isa => 'Num', required => 1;

around BUILDARGS => sub {
    my $orig = shift;
    my $class = shift;
    my %args = @_ == 1 ? %{$_[0]} : @_;
    my $q = $args{quantity};
    if ( $q != int $q ) {
        $args{name}    .= " ($q)";
        $args{price}   *= $q;
        $args{quantity} = 1;
    }
    return $class->$orig( %args );
};

sub itemprice { $_[0]->quantity * $_[0]->price }

sub as_string {
    return sprintf '%2u * %-40s (%7.2f) %8.2f', map $_[0]->$_,
    qw/quantity name price itemprice/;
}

package main;
use Test::More;

my $table = Article->new({ name => 'Table', quantity => 1, price => 199 });
is $table->itemprice, 199, $table->as_string;

my $chairs = Article->new( name => 'Chair', quantity => 4, price => 45.50 );
is $chairs->itemprice, 182, $chairs->as_string;

my $rope = Article->new( name => 'Rope', quantity => 3.5, price => 2.80 );
is $rope->itemprice, 9.80, $rope->as_string;
is $rope->quantity, 1, 'quantity set to 1';
is $rope->name, 'Rope (3.5)', 'name includes original quantity';

done_testing;

I'm wondering, however, whether there's a better idiom to do this in Moose. But maybe my question is all subjective and deserves swift closing. :-)

UPDATE based on perigrin's answer

I've adapted perigrin's code sample (minor errors, and 5.10 syntax) and tagged my tests onto the end of it:

package Article::Interface;
use Moose::Role;
requires qw(name quantity price);

sub itemprice { $_[0]->quantity * $_[0]->price }

sub as_string {
        return sprintf '%2u * %-40s (%7.2f) %8.2f', map $_[0]->$_,
        qw/quantity name price itemprice/;
}


package Article::Types;
use Moose::Util::TypeConstraints;
class_type 'Article::Internal';
class_type 'Article::External';
coerce 'Article::External' =>
  from 'Article::Internal' => via
{
        Article::External->new(
                name        => sprintf( '%s (%s)', $_->name, $_->quantity ),
                quantity    => 1,
                price       => $_->quantity * $_->price
        );
};


package Article::Internal;
use Moose;
use Moose::Util::TypeConstraints;
has name        => isa => 'Str', is => 'rw', required => 1;
has quantity    => isa => 'Num', is => 'rw', required => 1;
has price       => isa => 'Num', is => 'rw', required => 1;

my $constraint = find_type_constraint('Article::External');

=useless for this case
# Moose::Manual::Construction - "You should never call $self->SUPER::BUILD,
# nor"should you ever apply a method modifier to BUILD."
sub BUILD {
        my $self = shift;
        my $q = $self->quantity;
    # BUILD does not return the object to the caller,
    # so it CANNOT BE USED to trigger the coercion.
        return $q == int $q ? $self : $constraint->coerce( $self );
}
=cut

with qw(Article::Interface); # need to put this at the end


package Article::External;
use Moose;
has name        => isa => 'Str', is => 'ro', required => 1;
has quantity    => isa => 'Int', is => 'ro', required => 1;
has price       => isa => 'Num', is => 'ro', required => 1;

sub itemprice { $_[0]->price } # override

with qw(Article::Interface); # need to put this at the end


package main;
use Test::More;

my $table = Article::Internal->new(
        { name => 'Table', quantity => 1, price => 199 });
is $table->itemprice, 199, $table->as_string;
is $table->quantity, 1;
is $table->name, 'Table';

my $chairs = Article::Internal->new(
        name => 'Chair', quantity => 4, price => 45.50 );
is $chairs->itemprice, 182, $chairs->as_string;
is $chairs->quantity, 4;
is $chairs->name, 'Chair';

my $rope = Article::Internal->new(
        name => 'Rope', quantity => 3.5, price => 2.80 );
# I can trigger the conversion manually.
$rope = $constraint->coerce( $rope );
# I'd like the conversion to be automatic, though.
# But I cannot use BUILD for doing that. - XXX
# Looks like I'd have to add a factory method that inspects the
# parameters and does the conversion if needed, and it is always
# needed when the `quantity` isn't an integer.

isa_ok $rope, 'Article::External';
is $rope->itemprice, 9.80, $rope->as_string;
is $rope->quantity, 1, 'quantity set to 1';
is $rope->name, 'Rope (3.5)', 'name includes original quantity';

done_testing;

I agree it provides a better separation of concerns. On the other hand, I'm not convinced this is a better solution for my purpose, as it adds complexity and does not provide for an automatic conversion (for which I would have to add more code).


Solution

  • Based on the information you provided in the comments, you're actually modeling two different but related things. You've encountered the ugliness of trying to keep these two things as a single Class. You end up not properly separating your concerns and have ugly dispatch logic.

    You need to have two classes with a common API (a Role will enforce this) and a set of coercions to easily translate between the two.

    First the API is really straight forward.

     package Article::Interface {
            use Moose::Role;
    
            requires qw(name quantity price);
    
            sub itemprice { $_[0]->quantity * $_[0]->price }
    
            sub as_string {
                return sprintf '%2u * %-40s (%7.2f) %8.2f', map $_[0]->$_,
                qw/quantity name price itemprice/;
            }
     }
    

    Then you have a Class to represent your internal Articles, again this is pretty trivial.

     package Article::Internal {
          use Moose;
    
          has name => ( isa 'Str', is => 'rw', required => 1);
          has [qw(quantity price)] => ( isa => 'Num', is => 'rw', required => 1); 
    
          # because of timing issues we need to put this at the end
          with qw(Article::Interface);
     }
    

    Finally you have a class to represent your external articles. In this one you have to override some methods from the interface to deal with the fact that your attributes are going to be specialized[^1].

     package Article::External {
          use Moose;
    
          has name => ( isa 'Str', is => 'ro', required => 1);
          has quantity => ( isa => 'Int', is => 'ro', required => 1); 
          has price => (isa => 'Num', is => 'ro', required => 1);
    
          sub itemprice { $_[0]->price }
    
          # because of timing issues we need to put this at the end
          with qw(Article::Interface);
     }
    

    Finally you define a simple coercion routine to translate between the two.

    package Article::Types {
        use Moose::Util::TypeConstraints;
        class_type 'Article::Internal';
        class_type 'Article::External';
    
        coerce 'Article::Exteral' => from 'Article::Internal' => via {          
             Article::External->new(
                name => $_->name,
                quantity => int $_->quantity,
                price => $_->quantity * $_->price
             );
        }
    }
    

    You can trigger this coercion manually with:

    find_type_constraint('Article::External')->coerce($internal_article);
    

    Additionally MooseX::Types can be used for this last part to provide cleaner sugar, but I chose to stick with pure Moose here.

    [^1]: You may have noticed that I've made the attributes in the External article read-only. From what you've said these objects should be "consume only" but if you need the attributes to be writeable you'll need to define a coercion on quantity to deal with making sure that only Integers are stored. I'll leave that as an exercise to the reader.