push & pop, shift and unshift

[jez]

Hi everyone,

I am working on some perl stuff at the moment and it been a while so maybe i am being really stupid here, but...

I want to shuffle an array. i.e. take one item off the top and but it back on the bottom.

I thought that the difference between shift and pop was which end of the array they get the element from (and of course the same with unshift and push).

So my code is as follows....

sub randomSector{
@array = @_;
$arrayElementToMove = shift(@array);
push(@array, $arrayElementToMove);
my $retString = join(', ', @array);
return $retString;
}

But this seems to put it right back where i got it from.. not on the other end of the array.


Any help greatly appreciated!

Thanks

Jez

 

[PaulTEG]

sub randomSector{
    @array = @_;
    $arrayElementToMove = shift(@array);
    [b]reverse @array;[/b]
    push(@array, $arrayElementToMove);
    [b]reverse @array;[/b]
    my $retString = join(', ', @array);
    return $retString;
}

This could be what you're looking for? How big is the array?
--Paul

cigless ...

 

[jez]

Thanks for that,

The array is going to be a max of about 20 items.

Using the code as you suggested gave a warning of
"useless use of reverse in void context on line x"

So i tried moving it elsewhere in the function, but it doesnt have any effect.

Jez

 

[PaulTEG]

???
Which one is causing the problem?


cigless ...

 

[jez]

Both they simply have no effect.


Leaving them where they are gives two warnings, removing one gives one warning. Wherever i use the reverse function in that subroutine produces the message shown above. (also each value of $retString is in the same order.

 

[rharsh]

This works for me.

my @array = (0,1,2,3,4,5);
unshift(@array, pop(@array));
print @array, "\n";

my @array2 = (0,1,2,3,4,5);
push(@array2, shift(@array2));
print @array2, "\n";

 

[PaulTEG]

use strict;
my @array=(0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19);
print join (',',@array)."\n";
print &randomSector (@array)."\n";

sub randomSector{
    my @array = @_;
    my $arrayElementToMove = pop(@array);
    my @array2 = reverse @array;
    push(@array2, $arrayElementToMove);
    @array = reverse @array2;
    my $retString = join(', ', @array);
    return $retString;
}

Try the above, works for me

cigless ...

 

[fishiface]

reverse @array;

doesn't change @array - for that you'd need

@array = reverse @array;

That's why cigless Paul's fix doesn't do what you'd expect (how long is it now, Paul?).

shift() and pop() remove from an array - unshift() and push() add.
shift() and unshift() work on the beginning of an array, push() and pop() on the end. In other words, shift() and unshift() do the same thing to the left end of an array that push() and pop() do to the right end. Hence; to take from the left and put on the right,

my $tmp = shift @array;
push @array, $tmp;

or even

push @array, shift @array;

. I tried this

fish@spider:~$ cat t
#!/usr/bin/perl

my @a=1..5;
push @a, shift @a;
print join( ',', @a ), "\n";
fish@spider:~$ ./t
2,3,4,5,1
fish@spider:~$

and it works so I wonder whether your problem is elsewhere. Are you printing the array that you think you are printing?

f

"As soon as we started programming, we found to our surprise that it wasn't as easy to get programs right as we had thought. Debugging had to be discovered. I can remember the exact instant when I realized that a large part of my life from then on was going to be spent in finding mistakes in my own programs."
--Maurice Wilkes

 

[jez]

Thanks for the responses, I think fishiface has found the issue with ;-

" doesn't change @array - for that you'd need..."

I have an array read in from a text file that is simply a list of job titles. So each call to this subroutine should output that same list in a slightly different order.
There is no mission critical need for this, but i thought it should fairly straight forward.

The code that calls this subroutine is as follows

for($i=0;$i<@location;$i++){
	for($j=0;$j<@industrySectors;$j++){


open(OUTFILE, ">$outFileName") or 
die("Cant open output file: $outFileName: $!");
for($l=0;$l<@templateLines;$l++){
my $replacement = $templateLines[$l];
my $randomisedJobString = &randomSector(@industrySectors);

$pattern[0] = '\{\$currentLocation\}';
$pattern[1] = '\{\$jobString\}';
				
$replacement=~s/$pattern[0]/$currentLocation/;
$replacement=~s/$pattern[1]/$randomisedJobString/;

print OUTFILE $replacement;
}
close(OUTFILE);	


}
}

So I am trying to produce an output file for every combination of locations and job sectors.

I have tried to modify the subroutine to use some of the ideas above, but it doesnt seem to make a difference.

Could it be that the array being passed to the subroutine is not being altered? So that next time it is called it is in the same order (so any shuffling would produce the same result).

I DO want to alter the original array, no copies or temporary values.... do i need to pass a reference to the array? (i'm not hot on references, but i have a rough idea).


Thanks for your help so far.


Jez

 

[fishiface]

You've got it. Your passing the array as an argument list to &randomSector - which makes a copy - and then copying that again with "my @array = @_;"

Try

my $randomisedJobString = &randomSector(\@industrySectors);

and

sub randomSector{
    my $arrayRef = $_[0];
    push @$arrayRef, shift @$arrayRef;
    return join( ', ', reverse @$arrayRef );
}[code] so that you are munging the original rather than a copy of a copy.  It's good practise for performance reasons as well - the double copy really achieves nothing and would get slow with huge arrays.

Note also that[code]
for($l=0;$l<@templateLines;$l++){
   my $replacement = $templateLines[$l];
   ...

is rendered more perlishly as

foreach my $replacement (@templateLines) {
   ...

f





"As soon as we started programming, we found to our surprise that it wasn't as easy to get programs right as we had thought. Debugging had to be discovered. I can remember the exact instant when I realized that a large part of my life from then on was going to be spent in finding mistakes in my own programs."
--Maurice Wilkes

 

[fishiface]

oops - should have previewed!

on the pass-by-value/reference strand, using foreach actually aliases $replacement to each element of @templateLines in turn, whereas your for(; loop makes a copy each time. Unless you actually require a copy, foreach would be more efficient.

f

"As soon as we started programming, we found to our surprise that it wasn't as easy to get programs right as we had thought. Debugging had to be discovered. I can remember the exact instant when I realized that a large part of my life from then on was going to be spent in finding mistakes in my own programs."
--Maurice Wilkes

 

[jez]

my ($i, $j, $l, @location, @industrySectors, @templateLines, $line, $tmpLine, @pattern, $replacement,$jobString);

# get template file
my $templateFile = "testTemplate.html";

# get datafile
my $dataFile = shift;


# read in template file and parse with changes...
open(TEMPLATE, "$templateFile") or die("Cant open template file: $!");
@templateLines = <TEMPLATE>;
close(TEMPLATE);

# read in data...
open(FH, "$dataFile") or die("Cant open data file: $!");
while(<FH>){
chomp;              # no newline
    s/#.*//;                # no comments
    s/^\s+//;               # no leading white
    s/\s+$//;               # no trailing white
    next unless length;     # anything left?
    my ($var, $value) = split(/\s*=\s*/, $_, 2);
    if($var =~m/location/gi){ push @location, $value; }
    if($var =~m/industrySectors/gi){ push @industrySectors, $value;}
	}
close(FH);

for($i=0;$i<@location;$i++){
for($j=0;$j<@industrySectors;$j++){

# figure out output file name
my $outFileName = $location[$i] . "_" . $industrySectors[$j] .".html";	
my $currentLocation = $location[$i];
# write to output file
open(OUTFILE, ">$outFileName") or die("Cant open output file: $outFileName: $!");
foreach my $replacement(@templateLines){
my $randomisedJobString = &randomSector(\@industrySectors);
				
$pattern[0] = '\{\$currentLocation\}';
$pattern[1] = '\{\$jobString\}';
				
 				$replacement=~s/$pattern[0]/$currentLocation/;
$replacement=~s/$pattern[1]/$randomisedJobString/;
print OUTFILE $replacement;
		}
	close(OUTFILE);	
	}
}
	
sub randomSector{
    my $arrayRef = $_[0];
    push @$arrayRef, shift @$arrayRef;
    return join( ', ', reverse @$arrayRef );
}

exit(0);

that is the code in total.

I agree with you fishiface, it is definitely a pass-by-value issue, but it still outputs the joined string in the same order 20 times in a row. (for each iteration).
I cannot see what the problem can be anymore.

Jez

 

[fishiface]

Your logic seems a little broken. You're drawing the filename from the @industrySectors in sequence but then using a replacement string drawn at pseudo-random from the same array for the substitutions. Shouldn't the substitions match the filenames?

If this is really what you want, then rearranging an array while iterating over it will cause you no end of problems and I wouldn't like to guess the result. Make a copy of @industrySectors at the top and permute the copy while iterating over the original.

The two nested for loops sugest that you want every combination of location and sector, in which case you don't need the randomisation at all and you could get away with

foreach my $loc (@locations) {
    foreach my $isec (@industrySectors) {
        my $outFileName = "$loc\_$isec.html";
        local *OUTFILE; # will now auto-close on scope exit
        open(OUTFILE, ">$outFileName") or die "$0: $outFileName: $!";

        foreach my $replacement (@templateLines) {
            $replacement =~ s/\{\$location\}/$location/;
            $replacement =~ s/\{\$jobString\}/$isec/; 
            print OUTFILE $replacement;
        }
    }
}

If you want the randomisation then I don't understand why you need the $j loop.

In summary - not quite sure what you want but iterating over @industrySectors while permuting it inside the loop is a big no-no.

Yours,

f

"As soon as we started programming, we found to our surprise that it wasn't as easy to get programs right as we had thought. Debugging had to be discovered. I can remember the exact instant when I realized that a large part of my life from then on was going to be spent in finding mistakes in my own programs."
--Maurice Wilkes

 

[jez]

Yes i see what the problem is now.

You are right, there needs to be 2 copies of the industry sector array.. one for the file names and one for the randomisation.

The reason for the random and ordered use of the jobsector array was to get pages called e.g. London_banking.html and then in the page it will say something like "Banking Jobs in London... other jobs available in this area are.... [random string of job sectors]."

...

I have now made the changes (using a second array), and when i print that variable while the script runs it does shuffle the array.
BUT still the string comes out in the output file in the same order.


So the code that seems to be failing is the substitution

$randomisedJobString = &randomSector(\@movingIndustrySectors);
print $randomisedJobString;print "\n\n";
$pattern[0] = '\{\$currentLocation\}';
$pattern[1] = '\{\$jobString\}';
	
$replacement=~s/$pattern[0]/$currentLocation/;
$replacement=~s/$pattern[1]/$randomisedJobString/;

I dont see why the value of $randomisedJobString can be different between where it is printed (correctly) and what goes in the output file.

Jez

 

[fishiface]

Ha! You're working for a recruitment agency!

print "After extensive research, your perfect job is",
   random_job(), "\n";

Seriously though, can you post a bit of your template file that shows a variable in use?

f

"As soon as we started programming, we found to our surprise that it wasn't as easy to get programs right as we had thought. Debugging had to be discovered. I can remember the exact instant when I realized that a large part of my life from then on was going to be spent in finding mistakes in my own programs."
--Maurice Wilkes

 

[jez]

lol... close, a well known job search engine...

the template is very simple at the moment, i thought i would save the easy stuff until last.

The only reason i have used this particular format for the tags in the template is to match smarty templates and hopefully give my code a bit of a standard, but perhaps the dollar signs are causing problems? But they shouldnt be as it is only in the substitution that the dollars are used, and they are escaped in the regex.


I copied the array before the two loops start with the following.

@movingIndustrySectors = @industrySectors;

<html>
<title></title>

<body>
Here is some standard text included in the template<hr />
This is dynamic content for .... {$currentLocation} where they have jobs in the following categories
{$jobString}

</body>

</html>

 

[fishiface]

Hmmm. Looks like it's my fault. After persuading you to use

foreach my $replacement (@templateLines) {

'cos it looks more perlish, I've led you to corrupt the original @templateLines on the first pass, removing your placeholders. Try

foreach my $template (@templateLines) {
    my $outline = $template;
    foreach ($outline) { # alias $_
        s/\{\$location\}/$location/;
        s/\{\$jobString\}/$isec/; 
        print OUTFILE;
    }
}

The second foreach loop is a bit weird. I'm just using it to alias $_ to $outline so that I can use the simple forms of s/// and print, both of which act on $_ if not told otherwise. It's a short form of

foreach my $template (@templateLines) {
    my $outline = $template;
    $outline =~ s/\{\$location\}/$location/;
    $outline =~ s/\{\$jobString\}/$isec/; 
    print OUTFILE $outline;
}

and probably a matter of taste anyway.


Yours,

fish


"As soon as we started programming, we found to our surprise that it wasn't as easy to get programs right as we had thought. Debugging had to be discovered. I can remember the exact instant when I realized that a large part of my life from then on was going to be spent in finding mistakes in my own programs."
--Maurice Wilkes

 

[jez]

Thanks ;-)
It does now work.

I understand what you are saying, and i think i have learned a good amount about how arrays and loops actually work.. instead of just using them.


Many thanks to everyone who has answered on this.

Jez