Perl. Repetitive code inside OOP script

Question

I am building a script and need some help about repetitive code.

This is my constructor:

sub new {
    my $class = shift;
    my $self = {
                  ip          => shift,
                  username    => shift,
                  password    => shift,
                  need_enable => 0,
                  enable => '',
    };
    my $session = Net::Telnet->new( Timeout => 2, Errmode => 'return');         
    $self->{session} = $session;

   bless($self, $class);
}

Here are some methods:

sub _send_enable {
    my $self = shift;
    my $session = $self->{session};
    ...

sub _command {
    my $self = shift;
    my $session = $self->{session};
    my $command = shift;
    ...

sub _match_prompt {
    my $self = shift;
    my $session = $self->{session};
    my ( $prematchU , $matchU ) =
         $session->waitfor(match => '/(?m:^[\w.-]+\s?(?:\(config[^\)]*\))?\s?[\$#>]\s?(?:\(enable\))?\s*$)/',
                                        Timeout => 10);
    return  ($prematchU,$matchU);
}

As you can see, in those methods I need to recall the telnet instance initializated in the constructor.

I got more methods and in those methods I need to recall the telnet instance again.

Moreover, I need to call some methods inside a method. I always need to put the object as first argument:

_command($self, 'show privilege');

_check_enable($self);

May be I missing something about what I learnt in "Intermediate Perl", I just feel I can avoid all that repetitive code.

Thanks for any suggestion!

Re "I need to call some methods inside a method", That should be $self->_command('show privilege'); and $self->_check_enable();. What you have there aren't method calls. — ikegami
– ikegami, Commented Nov 27, 2013 at 18:47
Re "I just feel I can avoid all that repetitive code", You can delete it entirely if you want to. There's no need to copy the obj reference into a var. — ikegami
– ikegami, Commented Nov 27, 2013 at 18:50
There are several issues here: In your constructor, you're not returning your reference you've blessed. Nor are any of your methods returning anything... Or are they even closed. I've changed your tabs to spaces, so your code is readable (StackOverflow doesn't indent tabs). However, your code is missing some things. — David W.
– David W., Commented Nov 27, 2013 at 19:25
@David W., He is returning the blessed reference. I don't think those are the full methods; just a demo of the repetitive parts. — ikegami
– ikegami, Commented Nov 27, 2013 at 19:39
@DaveCross Yes. I keep forgetting that Perl returns the value of the last expression, and bless returns the blessed reference. You could create an entire constructor with just one statement: bless {}, shift;. — David W.
– David W., Commented Nov 28, 2013 at 15:16

friedo · Accepted Answer · 2013-11-27 19:55:13Z

First things first. This:

_command($self, 'show privilege');

_check_enable($self);

is very wrong. To call a method in Perl, you want to use

$self->_command( 'show privilege' );
$self->_check_enable();

(In the second line you can omit the parens since the argument list is empty.)

The -> operator tells Perl that you want to call a method instead of a regular subroutine. If the thing on the left side is a blessed reference, then the package into which it is blessed is used to lookup the method. Otherwise, the thing on the left is assumed to be a class name. In either case, Perl puts the thing on the left into the argument list as the first item.

The important difference between using -> and calling the subroutine directly is that the -> syntax causes Perl to lookup the method which may be in that class or a parent class. Calling a subroutine directly does not go through the method dispatcher.

As for accessing the object itself, the standard convention in Perl is to do

my $self = shift;

But it's not required. Since arguments to your method come in the regular @_ arguments array, you could just use $_[0]. You could also just use $_[0]{session} for your session object. But IMHO, it's neater and easier to read if you unpack it into a named variable.

If you want to avoid all the repetitive typing to define $self, you can use something like Method::Signatures which will give you a fancy method keyword that unpacks $self for you.

So this:

sub _command {
    my $self = shift;
    my $session = $self->{session};
    my $command = shift;
    ...

Becomes this:

method _command( $message ) { 
    # $self and $message are already defined, so just use them
    ...

tobyink · Accepted Answer · 2013-11-27 21:20:51Z

My module Moops is aimed at taking the repetition out of Perl OO. Here's an example...

use Moops;
use Net::Telnet;

class My::Class :ro {
  has ip          => ();
  has username    => ();
  has password    => ();
  has need_enable => (default => 0);
  has enable      => (default => '');
  has session     => (builder => 1);

  method BUILDARGS ($ip, $username, $password) {
    return {
      ip       => $ip,
      username => $username,
      password => $password,
    };
  }

  method _build_session () {
    Net::Telnet->new(Timeout => 2, Errmode => 'return');
  }

  method _send_enable () {
    my $session = $self->session;
    ...;
  }

  method _command ($command) {
    my $session = $self->session;
    ...;
  }

  method _match_prompt () {
    $self->session->waitfor(
      match   => '/(?m:^[\w.-]+\s?(?:\(config[^\)]*\))?\s?[\$#>]\s?(?:\(enable\))?\s*$)/',
      Timeout => 10,
    );
  }
}

David W. · Accepted Answer · 2013-11-27 21:53:52Z

You could eliminate my $self = shift;. After all, that's just $_[0]. You could simply use $_[0]->{session} instead of constantly doing my $session = $self->{session}. That would resolve some of the receptiveness too.

However, that repetitiveness is useful for documentation purposes. When I see my $self = shift; under the sub definition, I know immediately this is a method and not a constructor or a plain vanilla subroutine.

When I write methods and constructors, my first few lines fetch the parameters. I also hide the structure or my object from my constructors and methods. It does mean writing a few more lines of code, but that code helps document what I am attempting to do. It's a great way to catch possible errors, or to help the poor schlub who has to maintain and debug my code.

For example, here's how I would do the constructor:

sub new {
    my $class      = shift;
    my $ip         = shift;
    my $username   = shift;
    my $password   = shift;

    my $self = {};
    bless $self, $class;

    $self->ip($ip);
    $self->username($username);
    $self->password($password);
    $self->need_enable(0);
    $self->enable('');

    my $session = Net::Telnet->new( Timeout => 2, Errmode => 'return' );
    $self->session($session);

    return $self;
}

That's a lot more writing and maybe what you'd call repetition, but...

The first line tell you this is a constructor (my $class = shift;).
The next three lines tell you I expect three parameters in that order.
There's no indication how my object is actually stored. Instead, I use methods to set each field -- something many true object oriented languages insist upon. This may mean I have a lot more methods, but it also means that if I have to munge my class' structure, I don't have to modify each and every constructor or method in order to do it.

Here's what my session method would look like:

my session {
    my $self     = shift;
    my $session  = shift;

    if ( defined $session ) {
        if ( not $self->isa("Net::Telnet") ) {
            croak qq(Session parameter must be a "Net::Telnet" object.);
        }
        $self->{session} = $session;
    }
    return $self->{session};
}

This is the only place in my code where I mention how the session information is actually stored in my object. By creating a separate method to set/get my session, I can do a bit of testing. For example, I want to test whether or not session is a Net::Telnet object or not. And if I decide to change the way session is stored in my object, this is the only place in the code I have to touch.

Of course, now I have a lot more repetition you wanted to eliminate:

sub _command {
    my $self    = shift;
    my $command = shift;

    my $session = $self->session;
    ...

Taking my advice I gave at the beginning of this answer, you could eliminate a lot of repetition:

sub _command {
    my $command = $_[1];

    ... $_[0]->{session} ...  # Some line where you need session.

But, what would you gain? How easy would it be for some poor schlub to maintain your code? What if you change the way you store your session?. For example, you decide to use inside out objects. Imagine going through your code and finding things like $[0]->{session} and modify it. Talk about boring, error prone repetitious tasks!

However, all is not entirely lost. Instead of constantly repeating getter/setter methods like this:

sub some_method {
    my $self        = shift;
    my $some_method = shift;

    if ( defined $some_method ) {
       $self->{some_method} = $some_method;
    }
    return $some_method;
}

You can use the Class::Struct module. The nice thing about Class::Struct is that it's a standard Perl module. It's already installed.

Using Class::Struct eliminates most of the getter/setter method definitions. Instead, all you need to do is write the few methods where your code is actually doing something interesting. For example, one program I have had over 40 methods associated with it, but using Class::Struct brought it down to less than 10.

Another option is Moose which is very flexible, powerful, and probably the way Perl object usage is going. It hides a lot of the getting/setting stuff and the structure of your object. There's still a lot of controversy with Moose. It is big, slow, and bulky. It can be convoluted and complex too. There are many packages like Mouse that try to eliminate a lot of the issues while losing just a few of the more esoteric features.

Constructors refer to the functions that creates the class object and methods refer to the functions that operate on that class object. In strict OO languages like Java, new is a builtin function that creates the class object and there is a big distinction between methods and constructors. In Perl, new is merely another subroutine in the package. It's even possible in Perl to create a subroutine that acts as both a method and constructor. However, if you use OO parlance, it is still useful to distinguish between constructor and methods.

Carlitos_30 · Accepted Answer · 2013-11-28 21:46:46Z

0

Thanks very much for your time.

I changed the method call by now using the arrow operator (->).

I have no problem in writing more lines, but I wanted to be sure if that lines were essential or I was missing some OOP concept(Intermediate Perl teach about curing repetitive code).

I am gonna check the Moops module too.

Gonna read about how this forum works. I just added 4 whitespaces but the thing didn't indent how I expected.

Thanks very much!

answered Nov 28, 2013 at 21:46

Carlitos_30

3704 silver badges16 bronze badges

Collectives™ on Stack Overflow

Perl. Repetitive code inside OOP script

4 Answers 4

Comments

Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related