graq asked:

If I create a key,value pair in a hash(ref) which is stored in an array(ref) which is one of the methods on the object, will I need to rebuild the array and reset the property in order to effect the change?

Without actually running your code, everything appears to be correct. As for your question, it appears to be a bit vague. What do you mean 'create a key/value' pair? Are you referring to one of the pairs creating in the BEGIN block? Are you referring to updating a key/value pair for a particular supplier or or adding a new key/value pair on the fly? None of these should be particularly difficult.

In reading through your code, I'm not sure why you have the arrayref of hashrefs. It appears to me that this could be slow if you have many suppliers. The data structure that I am seeing looks like this (just the structure, I don't know the keys):

[ 
  { 
    'supplier_id' => 17,
    'item_foo'    => 3,
    'item_bar'    => 7
  },
  {
    'supplier_id' => 28,
    'item_foo'    => 5,
    'item_qux'    => 202
  }
]
[download]

Unless the array index actually conveys useful information, I would probably turn this into a hashref of hashrefs with the primary hashs keys being the supplier_id:

{
  '28' => {
            'item_foo' => 5,
            'item_qux' => 202
          },
  '17' => {
            'item_foo' => 3,
            'item_bar' => 7
          }
};
[download]

If this works for you, your &set_stock sub becomes simpler and faster:

sub set_stock
{
    my( $self, $supplier, $line, $stockLevel ) = @_;

    my $supplierList = ();
    my $supplierNumber = $supplier->supplier_id; 

    # $supplierList will be a reference to a hash of references to has
+hes.
    unless( $supplierList = $self->get('supplier_list') ) 
    {
        die( "Cannot set stock level when supplier list is corrupt: $s
+upplierList" );
    }

    my $reference = $line->reference;
    $supp->{ $supplierNumber }{ $reference } = $stockLevel;
}
[download]

Now that I've gotten thoroughly off-topic, what was your question again? :) If you can provide more detail of what problem you're trying to solve...

Cheers,
Ovid

Vote for paco!

Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

What do you mean 'create a key/value' pair?

I am refering to the hash for the new item=>stockLevel for a supplier.

I would probably turn this into a hashref of hashrefs with the primary hashs keys being the supplier_id

I think that is a good idea too, thanks :)

Now that I've gotten thoroughly off-topic, what was your question again?

By no means! You have hit the nail on the head.

If you can provide more detail of what problem you're trying to solve...


Hmm :)
The object is an order. Each order has a set of suppliers that can supply at least one of the items in the order. Each supplier should have a complete list of how many of each of the items in the order they have in stock.


Statement: Using the data structure and sub in your reply (assuming the syntax is correct), calling &set_stock will effect a permament change in the object properties without the need for calling the &set function.

My question (pretty much) was "Is the above statement correct?"

--
Graq

sub AUTOLOAD {

    my ($self,$newval) = @_;

    if ($AUTOLOAD =~ /.*::get_(\w+)/){
        return $self->{$1};
    }elsif($AUTOLOAD =~ /.*::set_(\w+)/){
        $self->{$1} = $newval;
    }else{
        croak "$AUTOLOAD is not a method";
    }

}
[download]

Interesting. That looks extremely useful and much better than my original code (and got a significant ++ing)

Do you know of any differences between the two (my original and your suggestion) in behaviour, performance, problems, restritions? Any conditions under which my original code is 'better' than your suggestion?

Or is your suggestion exactly what it looks like - much better all round :) ?

--
Graq

As an aside, you've chosen a rather roundabout way to make accessors with that string eval. Why not just use a closure:

sub _create_accessors
{
    my %orderObjectFields = @_;

    foreach my $field ( keys %orderObjectFields )
    {
        my $default = $orderObjectFields{$field};
        no strict 'refs';
        *{ __PACKAGE__ . "::$field" } = sub
        {
            my $value =
              ( scalar @_ > 1 ? $_[0]->{$field} = $_[1] : $_[0]->{$fie
+ld} );
            $value = $default unless defined $value;
            $value;
        };
    }
}
[download]

As an aside, you've chosen a rather roundabout way to make accessors with that string eval. Why not just use a closure:

I am, in a few months, going to be looking at various parts of the project and sprucing up the code. But I am wary of no strict 'refs'. I really do prefer to avoid using such methods. Your suggestion has given me ideas though, thanks :)

--
Graq

Since no strict 'refs' only affects the block it's in, it won't affect the rest of your program (past the next curly brace, anyway). I make a practice of putting it in the innermost scope possible. You can do the following instead if you're paranoid. Note that the no strict 'refs' only affects the one line with the assignment. Why would you hesitate to use that? I hope you're not absorbing the 'always use strict everywhere' dogma...

    foreach my $field ( keys %orderObjectFields )
    {
        my $default = $orderObjectFields{$field};
        my $closure = sub
        {
            my $value =
              ( scalar @_ > 1 ? $_[0]->{$field} = $_[1] : $_[0]->{$fie
+ld} );
            $value = $default unless defined $value;
            $value;
        };
        no strict 'refs'; # only affects next statement:
        *{ __PACKAGE__ . "::$field" } = $closure;
    }
[download]