views:

125

answers:

5

What is the best practise to solve this?

    if (... ) 
   { 
    push (@{$hash{'key'}}, @array ) ; 
     }
    else 
     {
     $hash{'key'} =""; 
 }

Is that bad practise for storing one element is array or one is just double quote in hash?

+2  A: 

It's probably simpler to use explicit array references:

my $arr_ref = \@array;
$hash{'key'} = $arr_ref;

Actually, doing the above and using push result in the same data structure:

my @array = qw/ one two three four five /;
my $arr_ref = \@array;
my %hash;
my %hash2;
$hash{'key'} = $arr_ref;
print Dumper \%hash;
push @{$hash2{'key'}}, @array;
print Dumper \%hash2;

This gives:

$VAR1 = {
          'key' => [
                     'one',
                     'two',
                     'three',
                     'four',
                     'five'
                   ]
        };
$VAR1 = {
          'key' => [
                     'one',
                     'two',
                     'three',
                     'four',
                     'five'
                   ]
        };

Using explicit array references uses fewer characters and is easier to read than the push @{$hash{'key'}}, @array construct, IMO.

Edit: For your else{} block, it's probably less than ideal to assign an empty string. It would be a lot easier to just skip the if-else construct and, later on when you're accessing values in the hash, to do a if( defined( $hash{'key'} ) ) check. That's a lot closer to standard Perl idiom, and you don't waste memory storing empty strings in your hash.

Instead, you'll have to use ref() to find out what kind of data you have in your value, and that is less clear than just doing a defined-ness check.

CanSpice
the else part is empty and if part is array .. what is best practise ...
Tree
+2  A: 

I'm not sure I understand your question, but I'll answer it literally as asked for now...

my @array = (1, 2, 3, 4);
my $arrayRef = \@array;     # alternatively: my $arrayRef = [1, 2, 3, 4];

my %hash;

$hash{'key'} = $arrayRef;   # or again: $hash{'key'} = [1, 2, 3, 4]; or $hash{'key'} = \@array;

The crux of the problem is that arrays or hashes take scalar values... so you need to take a reference to your array or hash and use that as the value.

See perlref and perlreftut for more information.


EDIT: Yes, you can add empty strings as values for some keys and references (to arrays or hashes, or even scalars, typeglobs/filehandles, or other scalars. Either way) for other keys. They're all still scalars.

You'll want to look at the ref function for figuring out how to disambiguate between the reference types and normal scalars.

Platinum Azure
Okay, someone care to explain the downvote?
Platinum Azure
I'm going out on a limb here (pun intended) by guessing it's because Tree didn't get exactly the answer he was looking for. I got a downvote on my answer as well. This is making me a lot more hesitant to answer any of Tree's questions from now on.
CanSpice
A: 

Dealing with the revised notation:

if (... ) 
{ 
    push (@{$hash{'key'}}, @array); 
}
else 
{
    $hash{'key'} = "";
}

we can immediately tell that you are not following the standard advice that protects novices (and experts!) from their own mistakes. You're using a symbolic reference, which is not a good idea.

use strict;
use warnings;

my %hash = ( key => "value" );
my @array = ( 1, "abc", 2 );
my @value = ( 22, 23, 24 );

push(@{$hash{'key'}}, @array);

foreach my $key (sort keys %hash) { print "$key = $hash{$key}\n"; }
foreach my $value (@array)        { print "array $value\n"; }
foreach my $value (@value)        { print "value $value\n"; }

This does not run:

Can't use string ("value") as an ARRAY ref while "strict refs" in use at xx.pl line 8.

I'm not sure I can work out what you were trying to achieve. Even if you remove the 'use strict;' warning, the code shown does not detect a change from the push operation.

use warnings;

my %hash = ( key => "value" );
my @array = ( 1, "abc", 2 );
my @value = ( 22, 23, 24 );

push @{$hash{'key'}}, @array;

foreach my $key (sort keys %hash)   { print "$key = $hash{$key}\n"; }
foreach my $value (@array)          { print "array $value\n"; }
foreach my $value (@value)          { print "value $value\n"; }
foreach my $value (@{$hash{'key'}}) { print "h_key $value\n"; }

push @value, @array;

foreach my $key (sort keys %hash) { print "$key = $hash{$key}\n"; }
foreach my $value (@array)        { print "array $value\n"; }
foreach my $value (@value)        { print "value $value\n"; }

Output:

key = value
array 1
array abc
array 2
value 22
value 23
value 24
h_key 1
h_key abc
h_key 2
key = value
array 1
array abc
array 2
value 22
value 23
value 24
value 1
value abc
value 2

I'm not sure what is going on there.

Jonathan Leffler
You get the error in the first one because you've already instantiated the `$hash{'key'}` value, so it becomes `"value"` when you refer to it. If it's not defined before you do the `push()`, Perl does what you want and does some crazy reference magic and makes an array reference as the value for `$hash{'key'}`.
CanSpice
@CanSpice: OK, I sort of see, through a glass darkly. I get more or less what you suggest with `my %hash = ();`. I know I don't want to know how it works; too contorted to be maintainable by me.
Jonathan Leffler
+1  A: 

I'm not sure what your goal is, but there are several things to consider.

First, if you are going to store an array, do you want to store a reference to the original value or a copy of the original values? In either case, I prefer to avoid the dereferencing syntax and take references when I can:

 $hash{key} = \@array;  # just a reference

 use Clone; # or a similar module
 $hash{key} = clone( \@array );

Next, do you want to add to the values that exist already, even if it's a single value? If you are going to have array values, I'd make all the values arrays even if you have a single element. Then you don't have to decide what to do and you remove a special case:

 $hash{key} = [] unless defined $hash{key};
 push @{ $hash{key} }, @values;

That might be your "best practice" answer, which is often the technique that removes as many special cases and extra logic as possible. When I do this sort of thing in a module, I typically have a add_value method that encapsulates this magic where I don't have to see it or type it more than once.

If you already have a non-reference value in the hash key, that's easy to fix too:

 if( defined $hash{key} and ! ref $hash{key} ) {
      $hash{key} = [ $hash{key} ];
      }

If you already have non-array reference values that you want to be in the array, you do something similar. Maybe you want an anonymous hash to be one of the array elements:

 if( defined $hash{key} and ref $hash{key} eq ref {} ) {
      $hash{key} = [ $hash{key} ];
      }
brian d foy
A: 

If your problem is how do you replace a empty string value you had stored before with an array onto which you can push your values, this might be the best way to do it:

if ( ... ) { 
    my $r = \$hash{ $key }; # $hash{ $key } autoviv-ed
    $$r   = [] unless ref $$r;
    push @$$r, @values;
}
else { 
    $hash{ $key } = "";
}
  • I avoid multiple hash look-ups by saving a copy of the auto-vivified slot.
  • Note the code relies on a scalar or an array being the entire universe of things stored in %hash.
Axeman