Faster method needed?

Steeeeeve has asked for the wisdom of the Perl Monks concerning the following question:

Replies are listed 'Best First'.
Re: Faster method needed? by MeowChow (Vicar) on Feb 06, 2001 at 23:16 UTC
Ignoring for now the larger issue of what exactly this code is supposed to do {g}, I would suggest that you fix the incorrect use of function prototypes. Prototypes are not checked in method invocations, and if you were to try to call your function directly, as in `Make_shiftedKey($self, $shiftval)`, you would generate a compile time error for having two arguments when your prototype specifies only one.	[reply] [d/l]
Re: Faster method needed? by dws (Chancellor) on Feb 06, 2001 at 22:41 UTC
Cache `$self->{S_KEY}` instead of going after it fresh each time. Likewise, cache and maintain information about stuff you know, such as the key length. Once that's sorted out, it'll be easier to look for less expensive ways to do the individual shifting/appending operations. `sub Make_shiftedKey ($) { my $self = shift; my $shift = shift; my $shiftIsOdd = $shift & 1; my $s_key = $self->{S_KEY}; my $length_s_key = length $s_key; if ( $shiftIsOdd ) { $s_key = substr($s_key, 1); } else { # $s_key = substr($s_key, 0, $length_s_key) - 1); chop($s_key); } $len_s_key--; foreach ( 1 .. ($shift - 1) % $length_s_key ) { { # my $key = substr($s_key, 0, 1); # $s_key = substr($s_key, 1); # $s_key .= $key; $s_key =~ s/^(.)(.)$/$2$1/; } $self->{S_KEY} = $s_key; return $s_key; }` [download] Unless I misunderstood or mistranslated your code, it leaves the key one character shorter. Is that what you intended? Update:* read the above as background, then skip directly to lemming's solution below.	[reply] [d/l] [select]
Re: Faster method needed? by lemming (Priest) on Feb 06, 2001 at 23:14 UTC
If I'm reading your code right: $self->{S_KEY} will be a string. $shift is how many characters to rotate If $shift is odd throw away first character `$self->{S_KEY} = substr($self->{S_KEY},1);` To the end of the str is default If $shift is even throw away last character Just use chop. Then shift the first $shift chars to the back The for loop calaculation is wasteful in that you do that calculation on every loop. (Unless the compiler optimizes it, but you're thinking C in construct) You could of done `foreach my $i (0..($shift %length($self->{S_KEY})))` I'm a bit () happy. Or better yet don't use a loop. `$shift = ($shift % length($self->{S_KEY}) $self->{S_KEY} = substr($self->{S_KEY}, $shift).substr($self->{S_KEY}, + 0, $shift);` [download] And as MeowChow points out, what's with the bad prototyping? I forgot I stripped that out of hand. use strict and turn on warnings	[reply] [d/l] [select]
Re: Faster method needed? by kschwab (Vicar) on Feb 06, 2001 at 22:40 UTC
This is begging for some explanatory text. What is the method supposed to do ?	[reply]
Re: Faster method needed? by Fastolfe (Vicar) on Feb 06, 2001 at 22:42 UTC
I re-wrote it so that it was a little more readable, but I still haven't a clue what it does. What is `$self->{S_KEY}`? What does it contain? What is `$shift`? Can you document the function's inputs and what you expect to get out of it? Some sample data might be helpful. `sub Make_shiftedKey ($) { my $self = shift; my $shift = shift; if ($shift % 2) { $self->{S_KEY} = substr($self->{S_KEY}, 1, length($self->{S_KE +Y})); } else { $self->{S_KEY} = substr($self->{S_KEY}, 0, length($self->{S_KE +Y}) - 1); } for (my $i=0; $i <= ($shift - 1) % length $self->{S_KEY}; $i++) { my $key = substr($self->{S_KEY}, 0, 1); $self->{S_KEY} = substr($self->{S_KEY}, 1, length($self->{S_KE +Y}) - 1); $self->{S_KEY} .= $key; } return $self->{S_KEY}; }` [download]	[reply] [d/l] [select]