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

Hello crew,

given a rainy weekend I played a bit with a module of mines. All went well till night but then some tests failed in a weird way. I was, inside the main module restantiate some object via new() using parameters stores inside a container of the father object and I had all check done to validate arguments passed to new failed for childrens..

the situation is similar to the following Exp.pm 

package Exp;

use 5.006;
use strict;
use warnings;
use Carp;

sub new{
    my $class = shift;
    my $validated = _validate( $_[0] );
    return bless {
        validated => $validated,
        stored      => [],
    },$class;
}
sub _validate{
    croak "No arg!" unless $_[0];
    return $_[0];
}
sub store {
    my $self = shift;
    my $it = _validate( $_[0] );
    push @{ $self->{ stored } }, new( $it );  # <---here NOT OK    
}
1;

[download]

If I use the module in the plain way all is well: 

# croaks without arguments

perl -I . -MData::Dump -MExp -e " $exp=Exp->new(); dd $exp "

No arg! at -e line 1.

# OK with args

perl -I . -MData::Dump -MExp -e " $exp=Exp->new(2); dd $exp "

bless({ stored => [], validated => 2 }, "Exp")

[download]

But if now I call the store methods, it fails: 


perl -I . -MData::Dump -MExp -e " $exp=Exp->new(2); $exp->store(3) "

No arg! at -e line 1.

[download]

By other hands i found the following workaround using bless{ validated => $it },__PACKAGE__ instead of the new call: see the ExpOk.pm module: 

package ExpOk;

use 5.006;
use strict;
use warnings;
use Carp;

sub new{
    my $class = shift;
    my $validated = _validate( $_[0] );
    return bless {
        validated => $validated,
        stored      => [],
    },$class;
}
sub _validate{
    croak "No arg!" unless $_[0];
    return $_[0];
}
sub store {
    my $self = shift;
    my $it = _validate( $_[0] );
    push @{ $self->{ stored } }, bless{ validated => $it },__PACKAGE__
+;  # <---here OK
}
1;

[download]

it works as (I) expected: 

perl -I . -MData::Dump -MExpOk -e " $exp=ExpOk->new(2); $exp->store(3)
+; dd $exp "

bless({ stored => [bless({ validated => 3 }, "ExpOk")], validated => 2
+ }, "ExpOk")

[download]

What happens with the first version of the module? Is my workaround a viable solution? Note that my intention is not to store children objects inside the father: in my project i store a configuration for various jobs to be executed and foreach one i instantiate a new object and i run it

thanks

L*

There are no rules, there are no thumbs..
Reinvent the wheel, then learn The Wheel; may be one day you reinvent one of THE WHEELS.

Replies are listed 'Best First'.
Re: calling new inside the same module?
by Corion (Patriarch) on Nov 26, 2018 at 12:55 UTC

    If you call the new subroutine, you need to give it a proper first argument:

     push @{ $self->{ stored } }, $self->new( $it );

    [download]
[reply]
[d/l]
[select]
      Thanks Corion

      anyway with push @{ $self->{ stored } }, $self->new( $it ); I get: Attempt to bless into a reference at Exp.pm line 11.

      L*

      There are no rules, there are no thumbs..
      Reinvent the wheel, then learn The Wheel; may be one day you reinvent one of THE WHEELS.
[reply]
[d/l]
[select]

        Then you need to either use (ref $self)->new(...) or have your constructor accept both, a reference or a string (that is, doing the my  $class = ref $self || $self;). Having such a dual-use constructor makes or at least made some people scream in agony, but I never really understood why.

[reply]
[d/l]
[select]

        In addition to what Corion already said, this is an option–

            push @{ $self->{ stored } }, __PACKAGE__->new( $it );

        Seems like an odd code design though. The container type is the contained type.

[reply]
[d/l]
Re: calling new inside the same module?
by choroba (Cardinal) on Nov 26, 2018 at 13:24 UTC
    What kind of object do you want to store? Note that running bless doesn't run the constructor, so in this case, the stored objects don't have the stored attribute (which is harmless, but for other classes, much more might be going on in the constructor).

    You could call new in the package itself. To specify the class, it's better to use ref $self than __PACKAGE__, so if you later inherit from the class, objects of the new class respect the class of the storing object. 

        push @{ $self->{stored} }, ref($self)->new($it);

    [download]

    It might be cleaner to let the user specify what class to use, so they can use various classes according to their needs (see Dependency Injection). It's not clear how validation should work in such a case, though: maybe the classes themselves should define a validate method? Or once the object's been constructed, it's guaranteed to be valid? Give us more details so we can answer (or ask) more questions.

    ($q=q:Sq=~/;[c](.)(.)/;chr(-||-|5+lengthSq)`"S|oS2"`map{chr |+ord
}map{substrSq`S_+|`|}3E|-|`7**2-3:)=~y+S|`+$1,++print+eval$q,q,a,

    [download]
[reply]
[d/l]
[select]
      Thank you all monks,

      > Note that my intention is not to store children objects inside the father: in my project i store a configuration for various jobs to be executed and foreach one i instantiate a new object and i run it

      So i have not to store children objects; i rather have an object that can do an action OR can load a serie of small configuration e instantiate children to do each one one action. Like:

      
my $obj = new->( arg => 23); 

$obj->run();  # run will use 23

# OR INSTEAD:

my $obj = new->( configuration => file ); # this will load [12, 23, 15
+] into $obj->{ container }

$obj->run_all();

sub run_all{
   my $self = shift;
   foreach my $it ( @{$obj->{ container }} ){
       my $new_obj = ref($self)->new($it);
       $new_obj->run;
   }
}
# update: i had Class instead of new in the above: corrected

      [download]

      It is that bad? I suppose no..

      I'd go with ref($self)->new($it) instead of __PACKAGE__->new($it) because seems more clear even if i have no intention to subclassing.. it is already a mess ;=)

      > Or once the object's been constructed, it's guaranteed to be valid?

      Yes, it is supposed to be like this; once instantiated the object is guaranteed to be valid.

      Anyway, now that the confusion is away from my mind, I know what i intended to write: infact new is a normal subroutine, but what i forgot is that it shift the package name!!

      What i intended to write was: new(__PACKAGE__,$it)

      L*

      There are no rules, there are no thumbs..
      Reinvent the wheel, then learn The Wheel; may be one day you reinvent one of THE WHEELS.
[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom