CODEWITHSUNDEEP
perlmethods
BarkerChippy
BarkerChippy

Reputation: 121

How can I ensure my method calls use the right method name on the right object?

I am working on a program which makes multiple attempts at processing, storing to a new log each time it tries (several other steps before/after).

use strict;
for (my $i = 0; $i < 3; $i++)
{
     my $loggerObject = new MyLoggerObject(tag => $i);
     #.. do a bunch of other things ..
     Process($loggerObject,$i);
     #.. do a bunch of other things ..
}
sub Process
{

    my ($logger,$thingToLog) = @_;
    sub Logger { $logger->Print($_[0]); }
    Logger("Processing $thingToLog");

}



package MyLoggerObject;

sub new
{
    my $package = shift;
    my %hash = (@_); my $self = \%hash;
    return bless $self, $package;
}

sub Print
{
   my $self = shift;
   my $value = shift;
   print "Entering into log ".$self->{tag}.": $value\n";
}

1;

To avoid having to do a bunch of $self->{logger}->Print() and risk misspelling Print, I tried to collapse them into the local subroutine as seen above. However, when I run this I get:

perl PerlLocalMethod.pl 
Entering into log 0: Processing 0
Entering into log 0: Processing 1
Entering into log 0: Processing 2

instead of:

perl PerlLocalMethod.pl 
Entering into log 0: Processing 0
Entering into log 1: Processing 1
Entering into log 1: Processing 2

I am presuming the problem is that the Logger method is 'compiled' the first time I call the Process method with the object reference I used on the first call but not afterwards. If I did $logger->Print(), misspelling Print, and hit a codepath I can't reliably test (this is for an embedded system and I can't force every error condition) it would error out the script with an undefined Method. I suppose I could use AUTOLOAD within logger and log any bad Method calls, but I'd like to know any other recommendations on how to make sure my Logger() calls are reliable and using the correct object.

Upvotes: 2

Views: 142

Answers (3)

choroba
choroba

Reputation: 242323

In Perl, subroutines are compiled during compile time. Embedding a named subroutine declaration into a subroutine doesn't do what one would expect and isn't recommended.

If you are afraid of typos, write tests. See Test::More on how to do it. Use mocking if you can't instantiate system specific classes on a dev machine. Or use shorter names, like P.

You can declare the Logger in the highest scope as a closure over $logger that you would need to declare there, too:

my $logger;
sub Logger { $logger->Print($_[0]) }

But it's confusing and can lead to code harder to maintain if there are many variables and subroutines like that.

Upvotes: 4

Georg Mavridis
Georg Mavridis

Reputation: 2341

As already explained above, using lexical defined variables in these kinds of method is not possible.

If you have to "duct-tape" this problem you could use global Variables (our instead of my).

sub Process
{

    our ($logger,$thingToLog) = @_;
    sub Logger { $logger->Print($_[0]); }
    Logger("Processing $thingToLog");

}

But be aware that $logger and $thingToLog are now global variables accessible outside this function.

Upvotes: 0

Dave Cross
Dave Cross

Reputation: 69314

If you had used use warnings in your code you would have seen the message:

Variable "$logger" will not stay shared at logger line 24.

Which would have alerted you to the problem (moral: always use strict and use warnings).

I'm not entirely sure why you need so many levels of subroutines in order to do your logging, but it seems to me that all of your subroutines which take the $logger object as their first parameter should probably by methods on the MyLoggerObject (which should probably be called MyLoggerClass as it's a class, not an object).

If you do that, then you end up with this code (which seems to do what you want):

use strict;
use warnings;

for my $i (0 .. 2) {
    my $loggerObject = MyLoggerClass->new(tag => $i);
    #.. do a bunch of other things ..
    $loggerObject->Process($i);
    #.. do a bunch of other things ..
}



package MyLoggerClass;

sub new {
    my $package = shift;
    my $self = { @_ };
    return bless $self, $package;
}

sub Process {
    my $self = shift;
    my ($thingToLog) = @_;

    $self->Logger("Processing $thingToLog");
}

sub Logger {
    my $self = shift;

    $self->Print($_[0]);
}

sub Print {
   my $self = shift;
   my ($value) = @_;

   print "Entering into log $self->{tag}: $value\n";
}

1;

Oh, and notice that I moved away from the indirect object notation call (new Class(...)) to the slightly safer Class->new(...). The style you used will work in the vast majority of cases, but when it doesn't you'll waste days trying to fix the problem.

Upvotes: 4

Related Questions