Beefy Boxes and Bandwidth Generously Provided by pair Networks
No such thing as a small change

Re: Perl Cipher & questions on semantics/layout optimisation.

by oko1 (Deacon)
on Feb 25, 2012 at 20:51 UTC ( #956143=note: print w/replies, xml ) Need Help??

in reply to Perl Cipher & questions on semantics/layout optimisation.

Hello, and welcome! It's always good to see someone bravely exposing their code to the merciless scrutiny... ahem. :) No, seriously - this is my favorite (and most effective, I think) way to learn; try out what you've come up with, and ask the experts for their comments. Might sting a bit at first, but you get a _big_ jump on knowledge.

Having said that, I'll start ripping off the Band-Aids. :) I'll remove some of the excessive doublespacing to make it a bit more readable.

$key=""; @karray= split(//,$key);

The above code actually does nothing useful. Since '$key' is empty, '@karray' will also be empty.

while (length($key) < 4) { print "Please Enter Your Desired Key (5 or more characters) \n --> + :"; $key=<>; }

Entering a 3-char long key will exit this loop. Since you're not using the chomp function, the key will always end in '\n' - and thus be 1 char longer than the number of characters you enter, which will pass your 'while()' test. This disagrees with your prompt. This also applies to all of your following user input routines.

for ($i=0;$i<length($key);$i++) { $temp = 0; for ($i2=0;$i2<length($key);$i2++) { $temp += (ord(@karray[$i])+ord(@karray[$i2])); $offsetpattern .= chr((ord(@karray[$i])+ord(@k +array[$i2])+ ord($offset2[$i % length($key)]))%256);

Since you don't have a variable called '@offset2', and '@karray' is empty, your code is fatally broken, and the rest of the it will not do what you expect it to do - so the rest of my comments will pertain to the general issues in your script rather than specifics.

As someone has already noted, your script isn't very "Perlish". This means more than you might think: the "Perlish" usages take effective advantage of the language's features, which gains more and more benefit as the script goes on. For example, you traverse arrays by getting the length of a string, decrementing that length, then laboriously counting (possibly incorrectly) from the first character to the last, and using that counter value to index into the array that you create by splitting the string:

$cyphertext = <>; $len = length($cyphertext)-1; @array = split(//, $cyphertext); for ($i=0;$i<$len;$i++) { $char = chr((ord(@array[$i])...

The Perlish way of getting both the index and the element would be more like this:

chomp(my $blah = <STDIN>); # This should be validated... my @input = split //, $blah; for my $index (0 .. $#input){ print "$index: $input[$index]\n"; }

Clean and simple. Just as another general comment, the three-term 'for' loop is fairly rare in Perl: there are a few uses for it, but it's usually not needed. It's much easier - and much more sensible, usually - to count things (that's what we humans usually do) than indices of things, which is a bit more abstract and requires an extra level of thought (and is thus a potential source of errors.)

for (my $i = 1; $i < 10; $i++){ print "$i\n"; # Ooops - only counted to 9! } for my $i (1..10){ print "$i\n"; # Does the right and obvious thing } print "$_\n" for 1..10; # Same thing, even more Perlish

Recommendation: grab a copy of Joseph Hall's "Effective Perl Programming". Good reading, including for beginners. Again, good on ya for asking for criticism so early on!

I hate storms, but calms undermine my spirits.
 -- Bernard Moitessier, "The Long Way"

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://956143]
and all is quiet...

How do I use this? | Other CB clients
Other Users?
Others surveying the Monastery: (2)
As of 2018-05-23 00:19 GMT
Find Nodes?
    Voting Booth?