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

I was doing some maintenance on some old perl code and came across an odd api. Seems that this module receives an object reference as part of it's API, then calls methods in the other object without actually "use'ing" the other ojbect. Is this good practice? Is there a better way?

Here's a cheesy (but accurate) example of the technique I am talking about:

#!/usr/bin/perl -w
use Data::Dumper;
use strict;

package Apple;

sub new {
    my $obj  = shift;
    my %self = ( color => 'red' );
    bless \%self, $obj;
}

sub serve {
    return 'sliced';
}


package Orange;

sub new {
    my $obj  = shift;
    my %self = ( color => 'orange' );
    if ( @_ ) {
        $self{apple} = shift;  ## other objects
        $self{shape} = shift;
    }
    bless \%self, $obj;
}

sub serve {
    my $self = shift;
    return ( $self->{apple} ) ? $self->{apple}->serve() : 'squeezed';
}


package main;

my $red = new Apple;
my $yel = new Apple;
$yel->{color} ='yellow';  ## or is it better to pass to the new() for 
+initialization.

my $foo = new Orange();
my $baz = new Orange( $red );  ## is it wise to pass an object ref vs.
+ "use'ing" the package


for my $obj ( $red, $yel, $foo, $baz ) {
    print 'served: ', $obj->serve, "\n";
}

print "\n fruit stand: \n"
    , Dumper($red)
    , Dumper($yel)
    , Dumper($foo)
    , Dumper($baz)
    , "\n";

[download]

Thanks for any guidance or recommendations!

Replies are listed 'Best First'.
Re: Perl OO best practice?
by ikegami (Patriarch) on Feb 08, 2010 at 20:17 UTC
    use is used to load a module. The module is already loaded, being in the same file you are already executing.

    Buy the way, I prefer 

    {
    package Apple;
    ...
}

{
    package Orange;
    ...
}

{
    ...
}

    [download]
    over 
    package Apple;
...

package Orange;
...

package main;
...

    [download]

    It provides better scoping for lexical variables and directives such as our @ISA.

[reply]
[d/l]
[select]

      yes ikegami, you are right, open/close braces are safer/better for inline packages. I should have qualified my example better: in my production code, the packages are in separate files.

      I squished them into one file for the sake of the example, in case someone wanted to run the code.

      :-)
      sjs

[reply]
        Then your code doesn't work without use. Why are you asking if omitting use is stylistically ok when your module doesn't even work when you omit it?
[reply]
[d/l]
[select]
Re: Perl OO best practice?
by Fletch (Bishop) on Feb 08, 2010 at 20:31 UTC

    Ditto the suggestion on wrapping in-file packages in their own blocks, and you also want to avoid the "indirect object" method call form (i.e. things like my $obj = new Foo; see The Problems with Indirect Object Notation for why).

    The cake is a lie.
    The cake is a lie.
    The cake is a lie.

[reply]
[d/l]
      Thanks for the URL, good reading!
[reply]
Re: Perl OO best practice?
by chromatic (Archbishop) on Feb 08, 2010 at 20:53 UTC
    Seems that this module receives an object reference as part of it's API, then calls methods in the other object without actually "use'ing" the other ojbect.

    Sure, that's fine. You want to be able to handle objects which implement a known API that you'll use without caring how they do so. If you couple the use of specific packages to the uses of objects of the given classes, you run the risk of violating that encapsulation.

[reply]
Re: Perl OO best practice?
by repellent (Priest) on Feb 09, 2010 at 07:27 UTC 
      package main;

my $yel = new Apple;
$yel->{color} ='yellow';

      [download]

    By modifying the internal hash reference $yel, we have subtly violated encapsulation. The main package should know nothing about ->{color}. However, it could know about an object's public interface - e.g., a setter:
        $yel->set_color('yellow');

    [download]

    A setter (potentially) allows us to expand our color-setting functionality. Say, if we wanted to log that we have set a particular color, we simply override and extend ->set_color. The previous approach does not allow this.

    The previous approach also tends to lead to bad growth of code. This is more apparent for larger codebases.

    So how to keep up with "OO best practice"? Easy! Just use Moose.
[reply]
[d/l]
[select]

      Moose is an excellent suggestion. 

      #!/usr/bin/perl

use strict;
use warnings;

my $red = Apple->new( color => 'green'  );
my $yel = Apple->new( color => 'yellow' );

$red->color( 'red' ); # It got ripe.

my $foo = Orange->new();
my $baz = Orange->new( apple => $red ); 

for my $obj ( $red, $yel, $foo, $baz ) {
    print 'served: ', $obj->serve, "\n";
}

print "\n fruit stand: \n",
    map $_->dump, $red, $yel, $foo, $baz;


BEGIN {
    package Apple;

    use Moose;
    use namespace::autoclean;

    has 'color' => (
        is => 'rw',
        isa => 'Str',
    );

    sub serve {
        return 'sliced';
    }

    __PACKAGE__->meta->make_immutable;
    1;
}

BEGIN {
    package Orange;

    use Moose;
    use namespace::autoclean;

    has 'apple' => (
        is => 'ro',
        isa => 'Apple',
        predicate => 'has_apple',
    );

    has 'shape' => (
        is => 'ro',
        isa => 'Str',
    );

    sub serve {
        my $self = shift;
        return $self->has_apple 
             ? $self->apple->serve() 
             : 'squeezed';
    }

    __PACKAGE__->meta->make_immutable;
    1;
}

      [download]

      I like to put all my extra packages in BEGIN blocks since it makes them as much work as much like they were used as possible. I also put the trailing true value in, so that I can grab them and dump them straight into a separate file if it should become desirable to break them out. This simple step has prevented me from having to rerun zillions of tests via make test and prove


      TGI says moo

[reply]
[d/l]
[select]

      Thanks chromatic and repellent! You both hit upon the same concern: violating encapsulation.

      I was most concerned about the odd syntax in the Orange::serve() method:

       31 sub serve {
 32     my $self = shift;
 33     return ( $self->{apple} ) ? $self->{apple}->serve() : 'squeeze
+d';
 34 }
 35

      [download]

      At first read, $self->{apple} looks like a hash ref, but then we are calling a method from it with the ->serve(). I have not come across that syntax, but it works.

      Your suggestions will help as I clean up this code.
      Thanks!
      sjs

[reply]
[d/l]
        • $self->{apple} is not a hashref. It returns the first object shifted in at: $self{apple} = shift;  ## other objects (hopefully)
        • $self is a blessed hashref.
        • $self->{apple}->serve() calls ->serve on the object returned by $self->{apple}
[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom