views:

228

answers:

3

Hello guys, I need some help regarding the arrays in Perl

This is the constructor I have.

BuildPacket.pm

     sub new {
            my $class = shift;    
            my $Packet = {
                _PacketName => shift,
                _Platform  => shift,
                _Version => shift,
                _IncludePath => [@_],
            };

            bless $Packet, $class;
            return $Packet;
        }

        sub SetPacketName {
            my ( $Packet, $PacketName ) = @_;
            $Packet->{_PacketName} = $PacketName if defined($PacketName);
            return $Packet->{_PacketName};
        }

       sub SetIncludePath {
            my ( $Packet, @IncludePath ) = @_;
            $Packet->{_IncludePath} = \@IncludePath;
        }

         sub GetPacketName {
            my( $Packet ) = @_;
            return $Packet->{_PacketName};
        }

        sub GetIncludePath {
           my( $Packet ) = @_;
           @{ $Packet->{_IncludePath} };
        }

(The code has been modified according to the suggestions from 'gbacon', thank you)

I am pushing the relative paths into 'includeobjects' array in a dynamic way. The includepaths are being read from an xml file and are pushed into this array.

# PacketInput.pm
if($element eq 'Include')
            {
             while( my( $key, $value ) = each( %attrs ))
                {
                if($key eq 'Path')
                    push(@includeobjects, $value);
                        }
                }

So, the includeobject will be this way:

@includeobjects = (
    "./input/myMockPacketName",
    "./input/myPacket/my3/*.txt",
    "./input/myPacket/in.html",
);

I am using this line for set include path

 $newPacket->SetIncludePath(@includeobjects);

Also in PacketInput.pm, I have

sub CreateStringPath
{
    my $packet = shift;
    print "printing packet in CreateStringPath".$packet."\n";
    my $append = "";
    my @arr = @{$packet->GetIncludePath()};
    foreach my $inc (@arr)
    {
        $append = $append + $inc;
        print "print append :".$append."\n";
    }
}

I have many packets, so I am looping through each packet

# PacketCreation.pl
my @packets = PacketInput::GetPackets();
foreach my $packet (PacketInput::GetPackets())
{
    print "printing packet in loop packet".$packet."\n";
    PacketInput::CreateStringPath($packet);
    $packet->CreateTar($platform, $input);
    $packet->GetValidateOutputFile($platform);
}

The get and set methods work fine for PacketName. But since IncludePath is an array, I could not get it to work, I mean the relative paths are not being printed.

+7  A: 

If you enable the strict pragma, the code doesn't even compile:

Global symbol "@_IncludePath" requires explicit package name at Packet.pm line 15.
Global symbol "@_IncludePath" requires explicit package name at Packet.pm line 29.
Global symbol "@_IncludePath" requires explicit package name at Packet.pm line 30.
Global symbol "@_IncludePath" requires explicit package name at Packet.pm line 40.

Don't use @ unquoted in your keys because it will confuse the parser. I recommend removing them entirely to avoid confusing human readers of your code.

You seem to want to pull all the attribute values from the arguments to the constructor, so continue peeling off the scalar values with shift, and then everything left must be the include path.

I assume that the components of the include path will be simple scalars and not references; if the latter is the case, then you'll want to make deep copies for safety.

sub new {
  my $class = shift;

  my $Packet = {
    _PacketName  => shift,
    _Platform    => shift,
    _Version     => shift,
    _IncludePath => [ @_ ],
  };

  bless $Packet, $class;
}

Note that there's no need to store the blessed object in a temporary variable and then immediately return it because of the semantics of Perl subs:

If no return is found and if the last statement is an expression, its value is returned.

The methods below will also make use of this feature.

Given the constructor above, GetIncludePath becomes

sub GetIncludePath {
  my( $Packet ) = @_;
  my @path = @{ $Packet->{_IncludePath} };
  wantarray ? @path : \@path;
}

There are a couple of things going on here. First, note that we're careful to return a copy of the include path rather than a direct reference to the internal array. This way, the user can modify the value returned from GetIncludePath without having to worry about mucking up the packet's state.

The wantarray operator allows a sub to determine the context of its call and respond accordingly. In list context, GetIncludePath will return the list of values in the array. Otherwise, it returns a reference to a copy of the array. This way, client code can call it either as in

foreach my $path (@{ $packet->GetIncludePath }) { ... }

or

foreach my $path ($packet->GetIncludePath) { ... }

SetIncludePath is then

sub SetIncludePath {
  my ( $Packet, @IncludePath ) = @_;
  $Packet->{_IncludePath} = \@IncludePath;
}

Note that you could have used similar code in the constructor rather than removing one parameter at a time with shift.

You might use the class defined above as in

#! /usr/bin/perl

use strict;
use warnings;

use Packet;

sub print_packet {
  my($p) = @_;
  print $p->GetPacketName, "\n",
        map("  - [$_]\n", $p->GetIncludePath),
        "\n";
}

my $p = Packet->new("MyName", "platform", "v1.0", qw/ foo bar baz /);
print_packet $p;

my @includeobjects = (
    "./input/myMockPacketName",
    "./input/myPacket/my3/*.txt",
    "./input/myPacket/in.html",
);
$p->SetIncludePath(@includeobjects);
print_packet $p;

print "In scalar context:\n";
foreach my $path (@{ $p->GetIncludePath }) {
  print $path, "\n";
}

Output:

MyName
  - [foo]
  - [bar]
  - [baz]

MyName
  - [./input/myMockPacketName]
  - [./input/myPacket/my3/*.txt]
  - [./input/myPacket/in.html]

In scalar context:
./input/myMockPacketName
./input/myPacket/my3/*.txt
./input/myPacket/in.html
Greg Bacon
I tried to implement the changes, and tried to print the array @{ $Packet->{_IncludePath} } from GetIncludePath, and it shows up nothing
superstar
@superstar The array will be empty initially because `my @includeobjects = ()` in the constructor creates a new, empty array. Code such as `$packet->SetIncludePath(qw/ foo bar baz /)` will give it an interesting value. Did you do this in your test?
Greg Bacon
I did not do it so far. I will try to implement right away!
superstar
I am actually using $newPacket->SetIncludePath(@includeobjects); in another module to pass the array of includeobjects, I should get these array values in GetIncludePath, which is not happening..
superstar
@brian d foy: when i call a $packet->GetIncludePath, i should be able to get the array values which has been set from SetIncludePath...right?
superstar
@superstar Take a look at the working example in the updated answer that stores and retrieves the include path. How is your code different?
Greg Bacon
@gbacon: thanks, that was in similar lines and your code works fine. But i have @other array to be ./input/myMockPacketName ./input/myPacket/my3/*.txt ./input/myPacket/in.html which are relative paths. so my include paths are specific and when i run with this as array, it doesnot work.
superstar
@superstar By doesn't work, do you mean it's not expanding the wildcard in the second path or something else? Please edit your question to show how you're using the class and the output you'd like to see so we can give you more helpful answers.
Greg Bacon
I enabled according to what you have specified. But the length of arr in CreateStringPath shows up as zero.
superstar
+3  A: 

Once you understand @gbacon's answer, you can save some typing by using Class::Accessor::Fast:

#!/usr/bin/perl

package My::Class;
use strict; use warnings;
use base 'Class::Accessor::Fast';

__PACKAGE__->follow_best_practice;
__PACKAGE__->mk_accessors( qw(
    IncludePath
    PacketName
    Platform
    Version
));

use overload '""' => 'to_string';

sub to_string {
    my $self = shift;
    sprintf(
        "%s [ %s:%s ]: %s",
        $self->get_PacketName,
        $self->get_Platform,
        $self->get_Version,
        join(':', @{ $self->get_IncludePath })
    );
}

my $obj = My::Class->new({
        PacketName => 'dummy', Platform => 'Linux'
});
$obj->set_IncludePath([ qw( /home/include /opt/include )]);
$obj->set_Version( '1.05b' );
print "$obj\n";
Sinan Ünür
+4  A: 

Another way to reduce typing is to use Moose.

package Packet;
use Moose::Policy 'Moose::Policy::JavaAccessors';
use Moose;

has 'PacketName' => (
    is       => 'rw',
    isa      => 'Str',
    required => 1,
);

has 'Platform' => (
    is       => 'rw',
    isa      => 'Str',
    required => 1,
);

has 'Version' => (
    is       => 'rw',
    isa      => 'Int',
    required => 1,
);

has 'IncludePath' => (
    is       => 'ro',
    isa      => 'ArrayRef[Str]',
    default  => sub {[]},
    traits => [ 'Array' ],
    handles => {
        getIncludePath       => 'elements',
        getIncludePathMember => 'get',
        setIncludePathMember => 'set',
    },
);

__PACKAGE__->meta->make_immutable;
no Moose;
1;

Check out Moose::Manual::Unsweetened for another example of how Moose saves time.

If you are adamant in your desire to learn classical Perl OOP, read the following perldoc articles: perlboot, perltoot, perlfreftut and perldsc.

A great book about classical Perl OO is Damian Conway's Object Oriented Perl. It will give you a sense of the possibilities in Perl's object.

daotoad
Class::Accessor is a reasonable alternative if Moos eis absolutely out of the question. But Moose rocks.
Ether
Thanks for the documentation and for the book suggested
superstar
Nothing against Moose, but I wouldn't say that one of its benefits is typing reduction. I think it's much too verbose and look forward to the day when it can generate the Moose code from a text description of the class.
brian d foy
@brian, that is an interesting idea. IME, Moose can be verbose, but it much more compact than writing out classical Perl OOP classes. Big savings come from types, coercion, method generation. But I would welcome a less verbose, punctuated syntax for object declaration.
daotoad