[30108] in Perl-Users-Digest

home help back first fref pref prev next nref lref last post

Perl-Users Digest, Issue: 1351 Volume: 11

daemon@ATHENA.MIT.EDU (Perl-Users Digest)
Mon Mar 10 16:20:07 2008

Date: Mon, 10 Mar 2008 13:19:58 -0700 (PDT)
From: Perl-Users Digest <Perl-Users-Request@ruby.OCE.ORST.EDU>
To: Perl-Users@ruby.OCE.ORST.EDU (Perl-Users Digest)

Perl-Users Digest           Mon, 10 Mar 2008     Volume: 11 Number: 1351

Today's topics:
        Updating single array element seems to send array out o <ed.whyatt@gmail.com>
    Re: Updating single array element seems to send array o <ed.whyatt@gmail.com>
    Re: Updating single array element seems to send array o <someone@example.com>
    Re: Updating single array element seems to send array o <someone@example.com>
    Re: Updating single array element seems to send array o <m@rtij.nl.invlalid>
    Re: Use of "return" in place of "last" (newbie question <tzz@lifelogs.com>
        Digest Administrivia (Last modified: 6 Apr 01) (Perl-Users-Digest Admin)

----------------------------------------------------------------------

Date: Mon, 10 Mar 2008 07:41:03 -0700 (PDT)
From: whyatt <ed.whyatt@gmail.com>
Subject: Updating single array element seems to send array out of kilter.
Message-Id: <0600edf3-b719-4247-a704-a40d42aefb4e@n75g2000hsh.googlegroups.com>

Hi all,

Forgive me for my perl inadequacies, but hopefully it is a very simple
solution that I am striving for.

The pseudocode of what I am attempting to do is as follows:
1. Read contents of a file and parse data into 2 arrays - person and
count.
2. Print out status of current arrays
3. Randomly choose one person from array 1 and increment the count by
1, updating the associated array element.
4. Print out new status of arrays.

The issue I face is that the print in task 2 nicely places each entry
on a single line, with no use of "\n".

After running task 3, the print in task 4 no longer places each entry
on a single line, rather causes lines to merge. I've scratched my head
for a day on this now, trying various different things with no luck.
Perhaps someone in this group can check my code and advise. (The input
file is a comma separated list)

Thanks in advance,
Ed

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+++++++++++++++++++++++

sub getsendersfile
{
	my $sendersfile = "icc_senders.csv";
	open(FILEHANDLE, $sendersfile) || die "Cannot open file. $!";
    until (eof FILEHANDLE)
    {
		return @allsenders = <FILEHANDLE>;
    }
	close FILEHANDLE;
}

sub readsendersfile
{
	@senderslist = ();
    @senderslistcount = ();
	for (my $count = 0; $count <= $#allsenders ; $count++)
	{
		$senderslist[$count] = ((split ",",$allsenders[$count])[0]);
		$senderslistcount[$count] = ((split ",",$allsenders[$count])[1]);
		print "$count : $senderslist[$count] : $senderslistcount[$count]";
	}
    print "\n";
    $numemp = $#senderslist + 1;
    print "$numemp\n";
}

sub choosesender
{
    foreach (1..10)
    {
    	#pick random sender
	    my @sender = ();
	    my $sendernumber = (int rand($numemp));
        $sender[0] = $senderslist[$sendernumber];
        my $newcount = $senderslistcount[$sendernumber];
        $newcount = ($newcount+1);
        splice @senderslistcount,$sendernumber,1,$newcount;
    }
	for (my $count = 0; $count <= $#allsenders ; $count++)
	{
		print "$count : $senderslist[$count] : $senderslistcount[$count]";
	}
}

getsendersfile();
readsendersfile();
choosesender();

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+++++++++++++++++

Results in my tests:

0 : tremlett@whyatt.local : 0
1 : anderson@whyatt.local : 0
2 : vaas@whyatt.local : 0
3 : aadams@whyatt.local : 0
4 : marshall@whyatt.local : 0
5 : iqbal@whyatt.local : 0
6 : gibbs@whyatt.local : 0
7 : maharoof@whyatt.local : 0
8 : mason@whyatt.local : 0
9 : langeveldt@whyatt.local : 0
10 : rehman@whyatt.local : 0
11 : lokuhettige@whyatt.local : 0
12 : cook@whyatt.local : 0
13 : panesar@whyatt.local : 0
14 : boucher@whyatt.local : 0
15 : chandana@whyatt.local : 0
16 : atapattu@whyatt.local : 0
17 : kallis@whyatt.local : 0
18 : jayasuriya@whyatt.local : 0
19 : collymore@whyatt.local : 0

20
0 : tremlett@whyatt.local : 11 : anderson@whyatt.local : 12 :
vaas@whyatt.local : 0
3 : aadams@whyatt.local : 0
4 : marshall@whyatt.local : 0
5 : iqbal@whyatt.local : 16 : gibbs@whyatt.local : 17 :
maharoof@whyatt.local : 0
8 : mason@whyatt.local : 0
9 : langeveldt@whyatt.local : 0
10 : rehman@whyatt.local : 111 : lokuhettige@whyatt.local : 112 :
cook@whyatt.local : 113 : panesar@whyatt.local : 0
14 : boucher@whyatt.local : 115 : chandana@whyatt.local : 116 :
atapattu@whyatt.local : 0
17 : kallis@whyatt.local : 0
18 : jayasuriya@whyatt.local : 0
19 : collymore@whyatt.local : 1


------------------------------

Date: Mon, 10 Mar 2008 09:10:21 -0700 (PDT)
From: whyatt <ed.whyatt@gmail.com>
Subject: Re: Updating single array element seems to send array out of kilter.
Message-Id: <60dc17ed-621b-498d-8a9a-b5d74317d387@m36g2000hse.googlegroups.com>

On 10 Mar, 15:35, Martijn Lievaart <m...@rtij.nl.invlalid> wrote:
> On Mon, 10 Mar 2008 07:41:03 -0700, whyatt wrote:
> > Hi all,
>
> > Forgive me for my perl inadequacies, but hopefully it is a very simple
> > solution that I am striving for.
>
> > The pseudocode of what I am attempting to do is as follows: 1. Read
> > contents of a file and parse data into 2 arrays - person and count.
> > 2. Print out status of current arrays 3. Randomly choose one person from
> > array 1 and increment the count by 1, updating the associated array
> > element. 4. Print out new status of arrays.
>
> > The issue I face is that the print in task 2 nicely places each entry on
> > a single line, with no use of "\n".
>
> Which is strange at first, where does the newline come from? It's tagged
> on to the count, because you don't chomp your input.
>
>
>
> > After running task 3, the print in task 4 no longer places each entry on
> > a single line, rather causes lines to merge. I've scratched my head for
> > a day on this now, trying various different things with no luck. Perhaps
> > someone in this group can check my code and advise. (The input file is a
> > comma separated list)
>
> You should have:
> - Posted (a small sample of) the inputfile
> - Posted a complete program, not hust the snipped you have trouble with.
>
> Without a complete program, it is difficult to check. Some random
> comments:
>
>
>
> > sub getsendersfile
> > {
> >    my $sendersfile = "icc_senders.csv";
> >    open(FILEHANDLE, $sendersfile) || die "Cannot open file. $!";
>
> Better use lexical filehandles.
>
>  open(my $fh, $sendersfile) or die "Cannot open '$sendersfile' for
> reading. $!";
>
> >     until (eof FILEHANDLE)
> >     {
> >            return @allsenders = <FILEHANDLE>;
> >     }
> >    close FILEHANDLE;
> > }
>
> This is nonsense, return, returns, so the loop is eecuted exactly once.
>
> Replace with:
>
>   # use a temporary to force list context
>   my @t = <FILEHANDLE>
>   return chomp(@t);
>
> However, you seem to set the global @allsenders as well. So what is the
> use of returning something?
>
>
>
> > sub readsendersfile
>
> This sub does not *read* the file, it parses it. So the name is
> misleading.
>
> > {
> >    @senderslist = ();
> >     @senderslistcount = ();
>
> Are these globals or locals? This part smells wrong.
>
> >    for (my $count = 0; $count <= $#allsenders ; $count++) {
>
> This is a C style loop, Perl has more effective mechanisms.
>
>
>
> >            $senderslist[$count] = ((split ",",$allsenders[$count])
> [0]);
> >            $senderslistcount[$count] = ((split ",",$allsenders
> [$count])[1]);
> >            print "$count : $senderslist[$count] : $senderslistcount
> [$count]";
> >    }
>
>         my %count;
>         for (@allsenders) {
>                 my ($email, $cnt) = split(/,/);
>                 $count{$email} = $cnt;
>                 print "$email: $cnt\n";
>         }
>
> You loose the numbering, but do you really need that?
>
> >     print "\n";
> >     $numemp = $#senderslist + 1;
> >     print "$numemp\n";
>
>         print "\n", scalar(keys(%count))"\n";
>
>
>
> > }
>
> > sub choosesender
> > {
> >     foreach (1..10)
> >     {
> >            #pick random sender
> >        my @sender = ();
> >        my $sendernumber = (int rand($numemp));
> >         $sender[0] = $senderslist[$sendernumber]; my $newcount =
> >         $senderslistcount[$sendernumber]; $newcount = ($newcount+1);
> >         splice @senderslistcount,$sendernumber,1,$newcount;
> >     }
> >    for (my $count = 0; $count <= $#allsenders ; $count++) {
> >            print "$count : $senderslist[$count] : $senderslistcount
> [$count]";
> >    }
> > }
>
> I'm not sure what this sub is supposed to do, and it is fairly
> unreadable. So I cannot comment on it.
>
>
>
> > getsendersfile();
>
> @allsenders = getsendersfile();
>
> > readsendersfile();
> > choosesender();
>
> I think you want something like this (untested):
>
> #!/usr/bin/perl
>
> use strict;
> use warnings;
>
> sub read_senders {
>         my $sendersfile = shift;
>         open(my $fh, $sendersfile)
>            or die "Cannot open '$sendersfile' for reading. $!";
>
>         # Read all lines, split every line on ',', stuff the result
>         # in an anonymous array and return an array with references
>         # to these anonymous arrays.
>         return map [ split /,/ ], <$fh>;
>
> }
>
> sub print_array {
>         print $_->[0], ": ", $_->[1], "\n"
>            for @_;
>
> }
>
> my @sendercount = read_senders("icc_senders.csv");
>
> # @senderscount now contains an array of arrayreferences
> # look up perldata for more info
>
> print_array(@senderscount);
>
> my $n = int rand(@senderscount);
> $sendercount[$n]->[1]++;
>
> print_array(@senderscount);
>
> HTH,
> M4

Thanks for all the feedback, I shall follow the suggestions and
hopefully see an improvement.

Cheers
Ed


------------------------------

Date: Mon, 10 Mar 2008 19:28:28 GMT
From: "John W. Krahn" <someone@example.com>
Subject: Re: Updating single array element seems to send array out of kilter.
Message-Id: <wZfBj.75260$FO1.71628@edtnps82>

whyatt wrote:
> 
> Forgive me for my perl inadequacies, but hopefully it is a very simple
> solution that I am striving for.
> 
> The pseudocode of what I am attempting to do is as follows:
> 1. Read contents of a file and parse data into 2 arrays - person and
> count.
> 2. Print out status of current arrays
> 3. Randomly choose one person from array 1 and increment the count by
> 1, updating the associated array element.
> 4. Print out new status of arrays.
> 
> The issue I face is that the print in task 2 nicely places each entry
> on a single line, with no use of "\n".

That is because you don't remove the "\n" when you read the file.


> After running task 3, the print in task 4 no longer places each entry
> on a single line, rather causes lines to merge.

When you add 1 to the second (last) field it removes the "\n" at the end 
and you don't add one yourself.


> I've scratched my head
> for a day on this now, trying various different things with no luck.
> Perhaps someone in this group can check my code and advise. (The input
> file is a comma separated list)
> 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++
> 
> sub getsendersfile
> {
> 	my $sendersfile = "icc_senders.csv";
> 	open(FILEHANDLE, $sendersfile) || die "Cannot open file. $!";
>     until (eof FILEHANDLE)

Why a loop?  In Perl you almost never need to use eof().


>     {
> 		return @allsenders = <FILEHANDLE>;

In list context <FILEHANDLE> returns all records from the file and 
subsequent reads from FILEHANDLE will return undef.  You are assigning 
that list to @allsenders and also returning that list from the 
subroutine, but you are calling the subroutine in void context so there 
is nowhere for the list to go.


>     }
> 	close FILEHANDLE;

This statement will never be reached because you return before you get here.


> }
> 
> sub readsendersfile
> {
> 	@senderslist = ();
>     @senderslistcount = ();
> 	for (my $count = 0; $count <= $#allsenders ; $count++)

That is usually written as:

	for my $count ( 0 .. $#allsenders )


> 	{
> 		$senderslist[$count] = ((split ",",$allsenders[$count])[0]);
> 		$senderslistcount[$count] = ((split ",",$allsenders[$count])[1]);
> 		print "$count : $senderslist[$count] : $senderslistcount[$count]";
> 	}
>     print "\n";
>     $numemp = $#senderslist + 1;

Or simply:

     $numemp = @senderslist;


>     print "$numemp\n";
> }
> 
> sub choosesender
> {
>     foreach (1..10)
>     {
>     	#pick random sender
> 	    my @sender = ();

@sender is lexically scoped so it can't be seen outside of this foreach 
loop so why are you using it?


> 	    my $sendernumber = (int rand($numemp));
>         $sender[0] = $senderslist[$sendernumber];
>         my $newcount = $senderslistcount[$sendernumber];
>         $newcount = ($newcount+1);
>         splice @senderslistcount,$sendernumber,1,$newcount;

That would be more simple and understandable as:

         #pick random sender
         my $sendernumber = int rand @senderslist;
         my @sender = $senderslist[ $sendernumber ];
         $senderslistcount[ $sendernumber ]++;


>     }
> 	for (my $count = 0; $count <= $#allsenders ; $count++)
> 	{
> 		print "$count : $senderslist[$count] : $senderslistcount[$count]";
> 	}
> }
> 
> getsendersfile();
> readsendersfile();
> choosesender();
> 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++

You probably want something like this:


my $sendersfile = 'icc_senders.csv';

open FILEHANDLE, '<', $sendersfile or die "Cannot open '$sendersfile' $!";

my @data;
while ( <FILEHANDLE> ) {
     chomp;
     push @data, [ split /,/ ];
     print $. - 1, " : $data[-1][0] : $data[-1][1]\n";
     }
close FILEHANDLE;

print scalar @data, "\n";

foreach (1..10) {
     #pick random sender
     my $sendernumber = int rand @data;
     my @sender = $data[ $sendernumber ][ 0 ];
     $data[ $sendernumber ][ 1 ]++;
     }

for my $count ( 0 .. $#data ) {
     print "$count : $data[$count][0] : $data[$count][1]\n";
     }



John
-- 
Perl isn't a toolbox, but a small machine shop where you
can special-order certain sorts of tools at low cost and
in short order.                            -- Larry Wall


------------------------------

Date: Mon, 10 Mar 2008 19:36:44 GMT
From: "John W. Krahn" <someone@example.com>
Subject: Re: Updating single array element seems to send array out of kilter.
Message-Id: <g5gBj.75262$FO1.56947@edtnps82>

Martijn Lievaart wrote:
> 
> On Mon, 10 Mar 2008 07:41:03 -0700, whyatt wrote:
>>
>>     until (eof FILEHANDLE)
>>     {
>> 		return @allsenders = <FILEHANDLE>;
>>     }
>> 	close FILEHANDLE;
>> }
> 
> This is nonsense, return, returns, so the loop is eecuted exactly once.
> 
> Replace with:
> 
>   # use a temporary to force list context
>   my @t = <FILEHANDLE>
>   return chomp(@t);

From 'perldoc -f chomp':

"It returns the total number of characters removed from all its arguments."

So you are not returning the contents of @t but the number of times that 
$/ was removed from @t.



John
-- 
Perl isn't a toolbox, but a small machine shop where you
can special-order certain sorts of tools at low cost and
in short order.                            -- Larry Wall


------------------------------

Date: Mon, 10 Mar 2008 16:35:36 +0100
From: Martijn Lievaart <m@rtij.nl.invlalid>
Subject: Re: Updating single array element seems to send array out of kilter.
Message-Id: <pan.2008.03.10.15.35.36@rtij.nl.invlalid>

On Mon, 10 Mar 2008 07:41:03 -0700, whyatt wrote:

> Hi all,
> 
> Forgive me for my perl inadequacies, but hopefully it is a very simple
> solution that I am striving for.
> 
> The pseudocode of what I am attempting to do is as follows: 1. Read
> contents of a file and parse data into 2 arrays - person and count.
> 2. Print out status of current arrays 3. Randomly choose one person from
> array 1 and increment the count by 1, updating the associated array
> element. 4. Print out new status of arrays.
> 
> The issue I face is that the print in task 2 nicely places each entry on
> a single line, with no use of "\n".

Which is strange at first, where does the newline come from? It's tagged 
on to the count, because you don't chomp your input.

> 
> After running task 3, the print in task 4 no longer places each entry on
> a single line, rather causes lines to merge. I've scratched my head for
> a day on this now, trying various different things with no luck. Perhaps
> someone in this group can check my code and advise. (The input file is a
> comma separated list)

You should have:
- Posted (a small sample of) the inputfile
- Posted a complete program, not hust the snipped you have trouble with.

Without a complete program, it is difficult to check. Some random 
comments:

> 
> sub getsendersfile
> {
> 	my $sendersfile = "icc_senders.csv";
> 	open(FILEHANDLE, $sendersfile) || die "Cannot open file. $!";

Better use lexical filehandles.

 open(my $fh, $sendersfile) or die "Cannot open '$sendersfile' for 
reading. $!";


>     until (eof FILEHANDLE)
>     {
> 		return @allsenders = <FILEHANDLE>;
>     }
> 	close FILEHANDLE;
> }

This is nonsense, return, returns, so the loop is eecuted exactly once.

Replace with:

  # use a temporary to force list context
  my @t = <FILEHANDLE>
  return chomp(@t);

However, you seem to set the global @allsenders as well. So what is the 
use of returning something?

> 
> sub readsendersfile

This sub does not *read* the file, it parses it. So the name is 
misleading.

> {
> 	@senderslist = ();
>     @senderslistcount = ();

Are these globals or locals? This part smells wrong.

> 	for (my $count = 0; $count <= $#allsenders ; $count++) {

This is a C style loop, Perl has more effective mechanisms.

> 		$senderslist[$count] = ((split ",",$allsenders[$count])
[0]);
> 		$senderslistcount[$count] = ((split ",",$allsenders
[$count])[1]);
> 		print "$count : $senderslist[$count] : $senderslistcount
[$count]";
> 	}

	my %count;
	for (@allsenders) {
		my ($email, $cnt) = split(/,/);
		$count{$email} = $cnt;
		print "$email: $cnt\n";
	}

You loose the numbering, but do you really need that?

>     print "\n";
>     $numemp = $#senderslist + 1;
>     print "$numemp\n";

	print "\n", scalar(keys(%count))"\n";
> }
> 
> sub choosesender
> {
>     foreach (1..10)
>     {
>     	#pick random sender
> 	    my @sender = ();
> 	    my $sendernumber = (int rand($numemp));
>         $sender[0] = $senderslist[$sendernumber]; my $newcount =
>         $senderslistcount[$sendernumber]; $newcount = ($newcount+1);
>         splice @senderslistcount,$sendernumber,1,$newcount;
>     }
> 	for (my $count = 0; $count <= $#allsenders ; $count++) {
> 		print "$count : $senderslist[$count] : $senderslistcount
[$count]";
> 	}
> }

I'm not sure what this sub is supposed to do, and it is fairly 
unreadable. So I cannot comment on it.

> 
> getsendersfile();

@allsenders = getsendersfile();

> readsendersfile();
> choosesender();

I think you want something like this (untested):

#!/usr/bin/perl

use strict;
use warnings;

sub read_senders {
	my $sendersfile = shift;
	open(my $fh, $sendersfile)
	   or die "Cannot open '$sendersfile' for reading. $!";

	# Read all lines, split every line on ',', stuff the result
	# in an anonymous array and return an array with references
	# to these anonymous arrays.
	return map [ split /,/ ], <$fh>;
}

sub print_array {
	print $_->[0], ": ", $_->[1], "\n"
	   for @_;
}

my @sendercount = read_senders("icc_senders.csv");

# @senderscount now contains an array of arrayreferences
# look up perldata for more info

print_array(@senderscount);

my $n = int rand(@senderscount);
$sendercount[$n]->[1]++;

print_array(@senderscount);

HTH,
M4


------------------------------

Date: Mon, 10 Mar 2008 09:44:50 -0500
From: Ted Zlatanov <tzz@lifelogs.com>
Subject: Re: Use of "return" in place of "last" (newbie question)?
Message-Id: <86pru2sknx.fsf@lifelogs.com>

On Sun, 09 Mar 2008 17:30:49 +0000 Michael <mike@nospam.co.uk> wrote: 

M> Just as a matter of interest, so far as I could see, my technique of 
M> setting a flag was the simplest way of maintaining the integrity of the 
M> foreach (terminating it with "last" rather than jumping out of it with 
M> "return"), whilst not wasting processor cycles performing irrelevant loops 
M> later in the function.  I am a newbie and don't know much - is there a 
M> more elegant alternative which would satisfy the functional programmers?

Don't worry about satisfying functional programmers or anyone else.  Get
the task done, write correct maintainable code, and that's enough.

Ted


------------------------------

Date: 6 Apr 2001 21:33:47 GMT (Last modified)
From: Perl-Users-Request@ruby.oce.orst.edu (Perl-Users-Digest Admin) 
Subject: Digest Administrivia (Last modified: 6 Apr 01)
Message-Id: <null>


Administrivia:

#The Perl-Users Digest is a retransmission of the USENET newsgroup
#comp.lang.perl.misc.  For subscription or unsubscription requests, send
#the single line:
#
#	subscribe perl-users
#or:
#	unsubscribe perl-users
#
#to almanac@ruby.oce.orst.edu.  

NOTE: due to the current flood of worm email banging on ruby, the smtp
server on ruby has been shut off until further notice. 

To submit articles to comp.lang.perl.announce, send your article to
clpa@perl.com.

#To request back copies (available for a week or so), send your request
#to almanac@ruby.oce.orst.edu with the command "send perl-users x.y",
#where x is the volume number and y is the issue number.

#For other requests pertaining to the digest, send mail to
#perl-users-request@ruby.oce.orst.edu. Do not waste your time or mine
#sending perl questions to the -request address, I don't have time to
#answer them even if I did know the answer.


------------------------------
End of Perl-Users Digest V11 Issue 1351
***************************************


home help back first fref pref prev next nref lref last post