I'm fairly certain you don't need the "unless (exists..." in put. Using "push @{$self->{$key}},$value" will auto-vivify $self->{$key} as an array ref if necessary.
--
Mike

• the subs in your package have prototypes which expect no arguments (although this won't effect them as methods since methods bypass prototypes)
• you don't need the unless statement in the put method as the hash value will be auto-vivified as an array reference via the push
• this may be better more naturally realised as a tied interface
• why wrap an object around a basic data type as it seems to be merely abstracting away dealing with references?

Apart from the above meanderings it seems fine if that's what you want from your object.
HTH

_________
broquaint

This object provides a simple front to a hash mapping keys to arrays of values thus allowing multiple values for a single key.

But it uses on OO interface, and hence you are missing out on all of the goodies hashes have to offer. No slices, no keys, no values, no each.

You are just offering a trivial object to store keys with many values in, not a replacement for hashes.

Abigail

"Life can only be understood in reverse, But must be lived forwards"

Regardless of tone, Abigail-II is correct. You should have based your object from Tie::Hash instead: (update: added DELETE)

package Tie::Hash::MultiVal;

use strict; 
use warnings;

use Tie::Hash;
use base qw(Tie::StdHash);

sub TIEHASH {
   my $class = shift;
   return bless {@_}, $class;
}

sub STORE {
   my ($hash,$k,$v) = @_;
   if (exists $hash->{$k}) {
      if (ref $hash->{$k}) {
         push @{$hash->{$k}}, $v;
      } else {
         $hash->{$k} = [$hash->{$k},$v];
      }
   } else {
      $hash->{$k} = $v;
   }
}

sub DELETE {
   my ($hash,$k) = @_;
   if (ref $hash->{$k}) {
      pop @{$hash->{$k}};
      $hash->{$k} = $hash->{$k}[0] if @{$hash->{$k}} == 1;
   } else {
      delete $hash->{$k};
   }
}

1;
[download]

Now i can use this to solve the 'problem' i had over at URI.pm bug or am i missing something? ... notice the slice ;)

use strict; 
use warnings;
use URI;
use Data::Dumper;
use Tie::Hash::MultiVal;

my $uri = URI->new('http://foo.com/bar.cgi?foo=bar&foo=baz&bar=qux&baz
+=qux');
my %hash;
tie %hash, 'MyHash';

%hash = $uri->query_form();
@hash{qw(foo bar baz qux)} = qw(one two three four);

delete $hash{foo};
print Dumper \%hash;
[download]

jeffa

L-LL-L--L-LL-L--L-LL-L--
-R--R-RR-R--R-RR-R--R-RR
B--B--B--B--B--B--B--B--
H---H---H---H---H---H---
(the triplet paradiddle with high-hat)

The object was never meant to be a replacement for hashes

That was (as I read it) Abigail's point. You're not providing a front end to a hash, you're providing an easy way of doing key -> arrayref mappings. You're dealing with a hashref not a hash. Hence your module description could be better expressed.

I think you read way to much into Abigail's post. Abigail's (usually pretty darn accurate) comments may be blunt, but the bluntness is always aimed at code, not people.

Unlike your message, which I --'d since I find it too close to a personal attack for my tastes.

The object is trivial. Nothing wrong with that in itself. However we have an expressive syntax in Perl already for this sort of thing.

my %map;
push @{$map{K1}}, 'V1';
push @{$map{K1}}, 'V2';
push @{$map{K2}}, 'V3';
push @{$map{K3}}, 'V4';
print Dumper(\%map);

# or

my %map2;
$map2{K1} = ['V1', 'V2'];
$map2{K2} = ['V3'];
$map2{K3} = ['V4'];
print Dumper(\%map2);

# or

my %map3;
@map3{'K1','K2','K3'} = (['V1', 'V2'], ['V3'], ['V4']);
print Dumper(\%map3);

# etc.
[download]

So, for me, it doesn't really supply anything that Perl doesn't give us already for free.

On a stylistic note I would use add_value() (or similar) rather than put() since the latter sounds like it should be the inverse of get() and it isn't.

arunhorne,
Abigail's is not that harsh. If you want to see a harsh critique
check out this Christmas socking stuffer. Hopefully a little
humor will defuse this situation. :)))

RTFM is quite often the best response we can give. Assuming we include which TFM and where to find it. If TFM contains the answer why should we waste time to write it again? Or copy it from the docs? If you get the pointer to the docs you will read the answer there and know where to look next time.

Of course if you say you do not get the answer from the docs it's something completely different. In that case we should try and give you a different explanation ... or point you to some other FM.

<sigh>To tell the truth the hardest questions to respond to are those that show the complete ignorance on the other party. Not those that show that the person did not care to look for the answer in any docs, but those that show he/she doesn't have any idea whay is he/she doing. Those that show that the person speaks a different language (no, I don't mean uninteligible translations from his/her mother language to English), that there is so huge a gap between his/her way of thinking and yours that there is simply no way to pass any information. In that case I really don't know what FM to suggest. Sometimes I think the person should start with Math for the second grade. (No that's not your case.)<sigh>

Jenda
Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
   -- Rick Osborne

Edit by castaway: Closed small tag in signature

Personally I concur with both Abigail-II and jeffa. They have already talked about what they think is not quite right, and I'll add a bit more.

Not only do we lose yummy hash goodness, but everytime we go to add data via this methodology, or remove data we are copying the entire array contents, allocate more memory, etc.... Granted on smaller data sets this might be trivial, but when the data sets get larger, and the hash structure gets sparser, this will lead to serious performance degredation.

I have had issues with some of Abigail-II's comments in the past as well, but 9 times out of 10 they are spot on technically, and it's just my ego getting in the way of me learning something new. I think that the whole point of the comments here were really just an attempt to show you that this particular wheel, isn't really a wheel and attempting to reinvent it may lead you to BadPlaces(TM). I would personally be interested to hear/read why you needed this abstraction.

No offense against your skill level coding, but this smacks of a learning process, looking back over my own personal experience. It feels like you may be making this abstraction to get around some other issue. More of a band-aid as opposed to a diagnosis and treatment. What can you use this as a base for that you couldn't with a normal hash or hash ref? Why a hash instead of an array, with elements you need being assigned numerical values? What problem space are you addressing?

One last thing. Personally I feel like you turned around and did to Abigail-II exactly what you claimed (he/she?) did to you. Personally I think the right thing to do would be to apologize to (him/her?), even if it isn't public as your comments came across as off the cuff, scathing, and down right immature..

Note: I'm simply asking you to look at the mirror, not attempting to start a flame war.

MMMMM... Chocolaty Perl Goodness.....

Further to my ramblings here about creating a hash that has multiple values per key I have gone the whole hog and written an object to perform this function having been advised that no such object currently exists.

I concur with the other heavy hitters that have weighed in on this thread already. Your module would never be included in code that I wrote, nor recommended by me in a publication I'm writing or class that I'm teaching. You are abstracting something that in this form does not need abstraction.

For an extreme example of the absurdity of this approach, check out the Meta hierarchy. Be sure you're sitting down while reading it, and not current swallowing any liquids.

-- Randal L. Schwartz, Perl hacker
Be sure to read my standard disclaimer if this is a reply.

A coworker once commented to me that the only explanation she could figure out for Meta was that it was some kind of subtle piece of performance art...