Refactoring a Moose Class

purinkle_2 has asked for the wisdom of the Perl Monks concerning the following question:

I am practising Kata Nine: Back to the CheckOut in Perl whilst also trying to use Moose for the first time.

So far, I've created the following class:

    package Checkout;

    # $Id$
    #

    use strict;
    use warnings;

    our $VERSION = '0.01';

    use Moose;
    use Readonly;

    Readonly::Scalar our $PRICE_OF_A => 50;

    sub price {
        my ( $self, $items ) = @_;

        if ( defined $items ) {
            $self->{price} = 0;

            if ( $items eq 'A' ) {
                $self->{price} = $PRICE_OF_A;
            }
        }

        return $self->{price};
    }

    __PACKAGE__->meta->make_immutable;

    no Moose;

    1;
[download]

The whole price method doesn't feel very Moose-ish and I feel like this could be refactored further.

Does anyone have any input on how this could be improved?

Comment on Refactoring a Moose Class Select or Download Code

Replies are listed 'Best First'.
Re: Refactoring a Moose Class by TomDLux (Vicar) on Oct 26, 2010 at 15:52 UTC
You've gotten off to the totally wrong tack. You say `use Mooose`, but then you proceed in a pre-Moose style, directly manipulating the `$self` hash. Presumably you want a price attribute, so describe it with Moose: `has 'price' => (isa => 'Int', is => 'rw', required => 1);` [download] Assuming you're requiring the user program to specify a price when they create a Checkout object. Or maybe it has a default price? or maybe you want a variety of classes: apples, oranges, bananas, cans of tomato soup, each of which has its own price. When you go up to the checkout with your shopping cart, the checkout processes the items one by one, asks it its price, and calculates the total. Details are good, but so is the overview. When you 'create the object', then think about how you're going to invoke the object, how you're goign to manipulate it. As Occam said: Entia non sunt multiplicanda praeter necessitatem.	[reply] [d/l] [select]
Re: Refactoring a Moose Class by Anonymous Monk on Oct 26, 2010 at 13:08 UTC
If the reason you're checking whether $items is defined is because you want it to return the total when it's called without args, then that's not Moose-y. Store subtotal, prices, and specials in attributes. Have `->scan` check whether an item exists in inventory (in this case: whether it has a price, but if you want to pretend there's another way to handle it, using a method for that check allows easy subclassing to override that). Then add the item's price, modulo special rules, to the total. Decide whether to die when encountering a bad item or to just complain loudly to STDERR and ignore the error otherwise -- the latter makes more sense in a cash register setting.	[reply] [d/l]