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

I'm in the process of building an object, which as one of it's accessors contains a list of other objects.

These other objects are only id/name pairs at the minute but may well grow, so I'm modelling them as proper objects from the start.

What are peoples thoughts on this as a design...

has _industries => ( is => 'rw', isa => 'HashRef[Foo::Industry]', auto_deref => 1, default => sub { {} }, clearer => 'clear_industries', traits => ['Hash'], handles => { has_industry_id => 'exists', add_industry => 'set', get_industry_for_id => 'get', industry_ids => 'keys', industries => 'values', num_industries => 'count', }, );

Clearly all the specified handles are how you'd interact with the list/hash.

I don't wanna get 3 months into using this code in various places only to realise a flaw in the design.

Any thoughts, comments, suggestions, issues?

Thanks
Cagao

Replies are listed 'Best First'.
Re: Moose Design Thoughts (Traits)
by moritz (Cardinal) on Dec 22, 2010 at 13:51 UTC

    You missed the most important part: the way this class is going to be used.

    In general good API design should include many possible use cases. Writing a class is just writing part of an API.

    As you've shown it, your class is just a glorified hash, with no reason not to use a hash in the first place. Adding methods that do some actual work might change the situation. But again that depends on how you want to use your class.

    Asking your questions without more background information is like asking "is the Ford Fiesta a good car for me?", but not telling what you want to use it for (offroad? crossing a dessert? taking your 6 family members to vacations? Or just some shopping half a mile away?)

      Cheers for the input.

      The object will be used by people I don't trust :) so I'm not giving them direct access to the hash, hence it also being "private" - it'll be used a lot by front-end guys working in TT, and thankfully that forces privacy of the methods.

      It'll also be used by people writing scripts (before/after TT), to set the contents of the hash before saving the state of it.

      This main class will have several of these types of lists, and other simpler attributes.

      I'd always previously just used a list in these circumstances, then moved to a hash for quicker lookups and less loops in checking the contents, and now using the Traits with Moose.

      I've added the trait handles as I've been writing a script that needs them, gradually moving more and more towards to the Moose way of doing things.

      Just wanted to make sure I wasn't going off on the wrong tangent, or maybe I've mis-understood something.

      Thanks again, all greatly appreciated.

        Ok, now you've told us who uses your class, but not how. Image you were not the implementer of the class, but its user. In your ideal world, what would you do with objects of that class, and how would the API look like?

Re: Moose Design Thoughts (Traits)
by stvn (Monsignor) on Dec 22, 2010 at 20:55 UTC

    Looks good to me, I do the same thing with the 'Hash' trait all the time myself. My only recommendation is to remove the 'auto_deref' as that feature is no longer recommended, instead use the 'elements' handler which will do the exact same thing.

    -stvn
      not the exact same thing, the same thing except for the context sensativity ... which is why auto_deref is no longer reccomended in the first place

      Good point, I started using the 'values' handler to grab a list of the values and shouldn't/won't be accessing the whole hash directly anyway.

      I set the contents in the class by passing in $self->_industries( \%selected ); but from then on using the trait handles.

      My previous implementations would have just stored the values in an array-ref, the move to using a hash is so I have quicker lookup with the ID of the values.

      My hash looks like this btw, incase it makes a difference...

      $obj->_industries( { 17 => { id => 17, name => 'foo' }, 26 => { id => 26, name => 'bar' }, } );

      There is obviously a duplication of the IDs, but that's minimal, and keeps the code that uses it simpler, rather than having to carry the ID AND the object around.

      That's the only down-side I can think of.

      btw, is the use of auto_deref no longer recommended in general, or only with Traits?

      I use it everywhere else with basic ArrayRef[] types.

        It is pretty much no longer recommended anywhere because it (ab)uses context and returns array/hash ref in scalar context and an array/hash in list context. While this is actually a fairly common idiom in Perl which I myself am long guilty of (ab)using (heck I even wrote the original auto-def code), it really is evil and should be stopped. The problem is that Perl context sensitivity can easily be the source of annoying and difficult to spot bugs, like for instance ...

        # normal usage, works just fine ... my @foo = $obj->foo; my $foo = $obj->foo; # but ... $other_obj->bar( foos => $obj->foo ); # whoops! # should actually be the more ugly ... $other_obj->bar( foos => scalar $obj->foo );
        While it might seem trivial, it really can be difficult to track down sometimes, especially when this is your primary accessor. But of course, it is nice to not have to manually de-ref all the time (Perl 5.13++) which is pretty much why the Hash trait and Array trait both have an 'elements' to delegate to that returns the derefed item in a non-context sensitive way.

        -stvn