0

I have a parameter object in Moose which has attributes of file wildcards to glob

So I had a method to do this

sub getInputFileParams{
    my ($self) = @_;

    #the only parameters passed in are in fact the input files
    return keys(%{$self->{extraParams}});
}

but then I though why not iterate the attributes as a hash?

has 'extraParams' => (
    is        => 'ro',
    isa       => 'JobParameters::Base',
    default   => sub { {} },
    traits    => ['Hash'],
    handles   => {
       keys_extraParams => 'keys',
    },

);

However that chokes as its not a hash reference. have I missed something or is using the object as a hash bad

KeepCalmAndCarryOn
  • 8,817
  • 2
  • 32
  • 47

2 Answers2

5

Yes, using objects as plain hashes is bad.

You're accessing their internal state directly, which bypasses any interface that they may present and makes your class closely coupled to the internal representation of the JobParameters::Base class.

If you need to be able to get the contents of a JobParameters::Base object as a hash, then add a to_hash method to JobParameters::Base, and delegate to that method in your attribute...

This means that if later you add caching (for example!) to JobParameters::Base, and use a __cache key to store internal data, you can safely make this change by also changing the to_hash method to remove the internal data from the hash it returns.

It is fine to store an attribute as just a hash, but if you're storing a blessed hash, then don't reach into it's guts..

t0m
  • 136
  • 2
  • I think this is the way forward. I am trying to write less arcane perl which is my getInputFileParams sub – KeepCalmAndCarryOn Feb 16 '12 at 15:38
  • More specifically to your question, write a method in JobParameters::Base that returns the stream of parameters that you want so you can iterate over that stream directly. – perigrin Feb 17 '12 at 00:33
-1

You've got all tools in place in your Moose class definition, you just aren't using them - try this:

return $self->keys_extraParams

RickF
  • 1,812
  • 13
  • 13
  • 1
    $self->extraParams->keys would require using Moose::Autobox, which is not always a good choice. – Stevan Little Feb 16 '12 at 15:31
  • Actually `keys` is a method he's getting from `traits => [Hash]` (Moose::Meta::Attribute::Native::Trait::Hash). No Autobox needed. – RickF Feb 16 '12 at 15:59
  • @RickF: no, native attribute traits still don't let you treat the hash as an object that you can call methods on. It defines methods *on the original object*, not the hash, so you would do `$self->keys` (although you should name the method something nicer, like extraParamKeys.) – Ether Feb 16 '12 at 16:48
  • Ah, I see my mistake now. I've removed the wrong bit - the `handles ...` line in the original code gives a functioning `keys_extraParams`. – RickF Feb 16 '12 at 18:16
  • This still won't do what the original poster wants because the handles in the original code is broken (unless he writes a keys() method in JobParameters::Base). – perigrin Feb 17 '12 at 00:35