Improved with a hatchet?

[Bill H]

I am using the following snippet of code to rotate a list starting at a
random postion and it seems to run real slow (it works the way I want
it to), can someone tell me how it can be improved?

sub RotateList
{
my $results = "";
my $srq = rand(@rotquest);
my $i = 0;
my $j = 0;
for($i = 0;$i < @rotquest;$i++)
{
$j = $srq;
$srq++;
if ($srq >= @rotquest){$srq = 0;}
$results .= $rotquest[$j];
}
return ($results);
}


And here is how I enter the routine:

@rotquest = (
qq~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED1" VALUE="An
automobile manufacturer or dealer ">An automobile manufacturer or
dealer<BR>~,
qq~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED2" VALUE="A
market research company">A market research company<BR>~,
qq~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED3" VALUE="A
newspaper or TV station">A newspaper or TV station<BR>~,
qq~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED4" VALUE="An
advertising agency">An advertising agency<BR>~);
$q4 = &RotateList();
$q4 .= qq~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED5"
VALUE="None of the above">None of the above\n~;

I am not concerned that it isn't returning a list since I just then
dump the results out to a web page.

Bill H www.ts1000.us

 

[anno4000]

I am using the following snippet of code to rotate a list starting at a
random postion and it seems to run real slow (it works the way I want
it to), can someone tell me how it can be improved?

sub RotateList
{
my $results = "";
my $srq = rand(@rotquest);
my $i = 0;
my $j = 0;
for($i = 0;$i < @rotquest;$i++)
{
$j = $srq;
$srq++;
if ($srq >= @rotquest){$srq = 0;}
$results .= $rotquest[$j];
}
return ($results);
}


And here is how I enter the routine:

[example snipped, useless because the output is missing]

I am not concerned that it isn't returning a list since I just then
dump the results out to a web page.

You could as well stringify the list and dump that. A routine that
promises to rotate a list should return a list, not a string. Also,
Perl has list manipulating functions that make the task very easy,
but those return lists.

While we're at it, a sub should also take parameters (the list to
rotate, the number of steps to rotate), not and not rely on global
variables or generate parameters internally. With these changes,
plus some internal cleanup your routine becomes

sub RotateList {
my ( $srq, @rotquest) = @_;
my @results;
for( 1 .. @rotquest )
{
my $j = $srq;
$srq++;
if ($srq >= @rotquest){$srq = 0;}
push @results, $rotquest[$j];
}
return (@results);
}

Your routine is slow because it moves each element at a time. Perl has
builtins that deal with lists and parts of lists as a whole. Since
rotation essentially means "cut the list in two and swap the parts"
They are more suited to the task. Here are some examples:

# using splice() to extract the parts and push() to swap them
sub RotateList1 {
my ( $srq, @rotquest) = @_;
push @rotquest, splice @rotquest, 0, $srq;
@rotquest;
}

# list slices to extract the parts, list concatenation to swap them
sub RotateList2 {
my ( $srq, @rotquest) = @_;
( @rotquest[ $srq .. $#rotquest], @rotquest[ 0 .. $srq - 1]);
}

# generate a list of swapped indices arithmetically. a list slice
# establishes the new order
sub RotateList3 {
my ( $srq, @rotquest) = @_;
@rotquest[ map $_ % @rotquest, $srq .. $srq + $#rotquest];
}

Anno

 

[Brian McCauley]

sub RotateList
{
my @list = @_;
my $index = int rand $#list;
return @list[$index+1 .. $#list], @list[0 .. $index];
}

Since we're optomising for speed I'd get rid of the redundant @list

sub RotateList
{
my $index = int rand $#_;
return @_[$index+1 .. $#list, 0 .. $index];
}

 

[Brian McCauley]

return splice(@list, $index+1), @list

Whilst that's likely to be fast and efficient and give the right result
it still gives me an uneasy feeling that it's behaviour is, strictly
speaking, undefined and could, at least in principle, change in a
future release.

 

[Brian McCauley]

sub RotateList
{
my @list = @_;
my $index = int rand $#list;
return @list[$index+1 .. $#list], @list[0 .. $index];
}

Since we're optomising for speed I'd get rid of the redundant @list

sub RotateList
{
my $index = int rand $#_;
return @_[$index+1 .. $#list, 0 .. $index];
}

Oops, the OP wanted a string... (oh, and there was a typo above).

sub RotateList
{
my $index = int rand $#_;
return join '', @_[$index+1 .. $#_, 0 .. $index];
}

Actually rather than pass a list to subrouine it's usually more
efficient to pass an arrayref.

sub RotateList
{
my $list = shift;
my $index = int rand $#$list;
return join '', @$list[$index+1 .. $#$list, 0 .. $index];
}

 

[Bill H]

At 2006-08-25 10:29AM, "Bill H" wrote:
it seems to run real slow (it works the way I want
it to), can someone tell me how it can be improved?

sub RotateList
{
my @list = @_;
my $index = int rand $#list;
return @list[$index+1 .. $#list], @list[0 .. $index];
}

Since we're optomising for speed I'd get rid of the redundant @list

sub RotateList
{
my $index = int rand $#_;
return @_[$index+1 .. $#list, 0 .. $index];
}

Oops, the OP wanted a string... (oh, and there was a typo above).

sub RotateList
{
my $index = int rand $#_;
return join '', @_[$index+1 .. $#_, 0 .. $index];
}

Actually rather than pass a list to subrouine it's usually more
efficient to pass an arrayref.

sub RotateList
{
my $list = shift;
my $index = int rand $#$list;
return join '', @$list[$index+1 .. $#$list, 0 .. $index];
}

Brian, I used your 1st example and will stick with a list instead of
making it a string. I have to make one change cause your example
wouldn't start on the 1st element (ie not rotate it). Here is what I
used:

sub RotateList
{
my $index = int rand $#_;
return join '', @_[$index .. $#_, 0 .. $index - 1];
}


Thanks to everyone for the help!

Bill H

 

[xhoster]

How is it undefined? The splice docs say:
Removes the elements designated by OFFSET and LENGTH
from an array, and replaces them with the elements
of LIST, if any. In list context, returns the
elements removed from the array.

Is the second occurence of @list in the return statement evaluated before
or after the splice is done to it? In other words, is the comma between
splice(...),@list as sequence point or not?

Xho

 

[John W. Krahn]

I am using the following snippet of code to rotate a list starting at a
random postion and it seems to run real slow (it works the way I want
it to), can someone tell me how it can be improved?

sub RotateList
{
my $results = "";
my $srq = rand(@rotquest);
my $i = 0;
my $j = 0;
for($i = 0;$i < @rotquest;$i++)
{
$j = $srq;
$srq++;
if ($srq >= @rotquest){$srq = 0;}
$results .= $rotquest[$j];
}
return ($results);
}


And here is how I enter the routine:

@rotquest = (
qq~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED1" VALUE="An
automobile manufacturer or dealer ">An automobile manufacturer or
dealer<BR>~,
qq~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED2" VALUE="A
market research company">A market research company<BR>~,
qq~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED3" VALUE="A
newspaper or TV station">A newspaper or TV station<BR>~,
qq~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED4" VALUE="An
advertising agency">An advertising agency<BR>~);
$q4 = &RotateList();
$q4 .= qq~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED5"
VALUE="None of the above">None of the above\n~;

I am not concerned that it isn't returning a list since I just then
dump the results out to a web page.

my @rotquest = (

q~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED1" VALUE="An automobile
manufacturer or dealer ">An automobile manufacturer or dealer<BR>~,
q~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED2" VALUE="A market research
company">A market research company<BR>~,
q~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED3" VALUE="A newspaper or TV
station">A newspaper or TV station<BR>~,
q~ <INPUT TYPE=CHECKBOX NAME="EMPLOYED4" VALUE="An advertising
agency">An advertising agency<BR>~
);

print splice( @rotquest, rand @rotquest ), @rotquest, q~ <INPUT
TYPE=CHECKBOX NAME="EMPLOYED5" VALUE="None of the above">None of the above\n~;



John

 

[Ben Morrow]

Quoth (e-mail address removed):

How is it undefined? The splice docs say:

[snip]

Is the second occurence of @list in the return statement evaluated before
or after the splice is done to it? In other words, is the comma between
splice(...),@list as sequence point or not?

As perl is currently implemented, it is. The scalar and list comma
operators are actually the same underneath (a generic evaluate-and-build
-a-list op), and the main use of the comma op in scalar context is as a
sequence point. Now, there's obviously no official spec for Perl that I
can wave at you , but I seriously doubt p5p would change something
like that without a major amount of fuss.

All of this only goes for Perl5, of course. Perl6 may well change
things, but, well, that's kinda the point .

Ben

 

[Brian McCauley]

As perl is currently implemented, it is. The scalar and list comma
operators are actually the same underneath (a generic evaluate-and-build
-a-list op), and the main use of the comma op in scalar context is as a
sequence point.

That is true but to say that the list context comma is a sequence point
is a gross over- simplification...

my $foo = 1;
print $foo,$foo++; # prints 21

my @list = ('foo','bar');
print @list,do { $_=uc for @list; @list }; # prints FOOBARFOOBAR

It happens that the comma causes an array on its LHS to be resolved to
a list of lvalues before the RHS is evaluated but it does not cause
those lvalues to be resolved to rvalues.

It would be a relatively small future change for the comma operator to
leave the array unresolved.