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

While reading someones code, I came across this function used by a constructor to initialize an object.
Can anyone see what the author intended to do here. To me, seems confusing and not very feasable!! 

# Description   : _init private method, should only be called by new t
+o initialize a new object
# Operation     : $self->init(@array);
#
######################################################
                                                                      
+       
sub _init {
    my $self = shift;
    if (@_) {
        my %initdata = @_;
        @$self{keys %initdata} = values %initdata;
    }
}

[download]

Replies are listed 'Best First'.
Re: Make sense?
by jettero (Monsignor) on Jul 19, 2007 at 15:57 UTC
    Looks fine to me. It's rather similar to... 
    sub _init {
    my $this = shift;
    my $that = { @_ };
    
    while( my ($k, $v) each %$that ) {
        $this->{$k} = $v;
    }
}

    [download]

    ... but it uses hash slices instead. Does that make more sense?

    -Paul

[reply]
[d/l]
      Of course. Thanks very much :).
      Forgetting about hash slices, the @{$self} led me to believe I was dereferencing some array.
[reply]
Re: Make sense?
by jdporter (Paladin) on Jul 19, 2007 at 16:01 UTC

    Certainly. Though there's really no need for the if. 

    sub _init {
    my( $self, %initdata ) = @_;
    @{$self}{keys %initdata} = values %initdata;
}

    [download]
    A word spoken in Mind will reach its own level, in the objective world, by its own weight
[reply]
[d/l]
[select]
Re: Make sense?
by afresh1 (Hermit) on Jul 19, 2007 at 19:47 UTC

    The only thing I would worry about is that there is no guarantee that keys or values called on %hash more than once will return them in the same order. And keys %hash and values %hash might not return them in the same order

    At least that is what I understood. If I am wrong, please tell me. Or is this something that "works" but isn't "right"?

    I would probably (untested)

    sub _init {
    my ($self, %initdata) = @_;
    if (%initdata) {
        map { $self{$_} = $initdata{$_} } keys %initdata;
    }
}

    [download]

    l8rZ,
    --
    andrew
[reply]
[d/l]

      From the docs for keys:

      The keys are returned in an apparently random order. The actual random order is subject to change in future versions of perl, but it is guaranteed to be the same order as either the values or each function produces (given that the hash has not been modified).

      For a given hash, as long as it's not changed, keys, values and each will return keys and/or values in the same order.

[reply]
[d/l]
[select]
      afresh1 .. You probably meant that to be 
             map { $self->{$_} = $initdata{$_} } keys %initdata;

      [download]
      since there is no indication of the existance of a %self hash, or the relevance of such a hash to the object.
      Also, there is not need for the "if" since empty "keys" will effectively convert the map to a no-op.
      So your code is equivalent to the hash slice jdporter wrote.

      My preference would be to simplify it to : 

        %$self =  (%$self, %initdata);

      [download]
      which, although representing infinitasmally more overhead, has an elegant simplicity to it.

      If you really wanted to be non-destructive of $self or $_[0], you could: 

         $self->{$_}=$initdata{$_} for keys %initdata;

      [download]
      which, to me, is slightly more readable/DWIM, compared to a "map" whose result is thrown away.

           "An undefined problem has an infinite number of solutions." - Robert A. Humphrey         "If you're not part of the solution, you're part of the precipitate." - Henry J. Tillman

[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom