[cloudnyn3]

Ok so I'm working on a script that a friend of mine wrote and what it is, is an anagram descrambler.
The point is to take a list of scrambled words from a given wordlist and then descramble them in under 30 seconds and enter them back into a form to be checked.
The problem I'm having is that for some reason it goes to the end of the list and uses the last word and returns it no matter the given input, I checked sytax and made sure that all of the curly brackets are where they're supposed to be but it still doesn't function properly.
As you will see I'm new to perl, its simlar to php so its a little easier but I still don't know all the rules =/
If you could help I'dappreciate it thanks!

#!/usr/local/bin/perl
my $filename = "/wordlist.txt";
my %wordlist;
my @curlist;
my $strid;
my $wrd;
my @inlist;
open( LIST, ") {
@curlist = (0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,

0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,

0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,

0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,

0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,

0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
chop;
s/\r//;
$word = $_;
foreach (split(//)) {
$curlist[ord]++;
}
$strid = join(", @curlist);
$wordlist{$strid} = $word;
#	print $word . "\t" . $strid . "\n";

close(LIST);
print "word:";
while () {
chop;
@inlist = ();
foreach (split(/,/)) {
@curlist = (0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,

0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,

0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,

0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,

0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,

0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
$wrd = $_;
foreach (split(//)) {
$curlist[ord]++;}
}
$strid = join(", @curlist);
if (exists($wordlist{$strid})) {
print $wordlist{$strid} . "\n";
push @inlist, $wordlist{$strid};
} else {
print "*** " . $wrd . ": not found\n";
push @inlist, '-';
}
}
print join(',', @inlist);

This is the error i keep getting, but when I fix it or try to it doesn't work =/

C:\Perl\bin>ana.pl
Backslash found where operator expected at C:\Perl\bin\ana.pl line 53, near "pri
nt $wordlist{$strid} . "\"
  (Might be a runaway multi-line "" string starting on line 51)
		(Do you need to predeclare print?)
String found where operator expected at C:\Perl\bin\ana.pl line 56, near "print
""
  (Might be a runaway multi-line "" string starting on line 53)
		(Missing semicolon on previous line?)
Backslash found where operator expected at C:\Perl\bin\ana.pl line 56, near "fou
nd\"
String found where operator expected at C:\Perl\bin\ana.pl line 56, at end of li
ne
		(Missing semicolon on previous line?)
syntax error at C:\Perl\bin\ana.pl line 53, near "print $wordlist{$strid} . "\"
syntax error at C:\Perl\bin\ana.pl line 56, near "" . $wrd . ":"
Can't find string terminator '"' anywhere before EOF at C:\Perl\bin\ana.pl line
56.

This post has been edited by cloudnyn3: 19 August 2009 - 02:02 PM

[dsherohman]

cloudnyn3, on 19 Aug, 2009 - 08:58 PM, said:

The problem I'm having is that for some reason it goes to the end of the list and uses the last word and returns it no matter the given input, I checked sytax and made sure that all of the curly brackets are where they're supposed to be but it still doesn't function properly.

I don't see how it could produce incorrect output if it won't even compile, so let's take a look and get it runnable first, shall we?

#!/usr/local/bin/perl

General suggestion: Always start your Perl programs with "use strict; use warnings;". These directives will save you from much weeping and gnashing of teeth. (There are times to not use them, but this is a good rule to follow until you are experienced enough to recognize when and why it should be ignored.)

open( LIST, ") {

This is the primary reason your code won't compile. You used a double-quote there (") rather than two single-quotes (''). This causes ") { @curlist = [...and so on up until...] = join(" to be treated as a single string and (if it were to compile and run) the "open" would look for a file with that string as its name.

I suspect what you wanted to do here was probably

open(LIST, $filename);

However, you should check for errors, the two-argument form of open is deprecated, and lexical filehandles are preferred these days, so

open(my $list, '<', $filename) or die "Failed to open $filename: $!\n";

would be a much better way to do it.

You also appear to have been expecting open to iterate over the lines in the opened file. It doesn't. It just opens the file and does not automatically read any of the file's data, since it has no idea what you want to do with the data or how it might be appropriate to read it.

@curlist = (0, 0,...

Ouch... I'm not entirely sure why you're building an array containing 508 zeroes, but Perl provides the "x" operator to remove the pain of doing so:

@curlist = (0) x 508;

chop;
s/\r//;
$word = $_;
foreach (split(//)) {
$curlist[ord]++;
}

Since open doesn't iterate through the lines in the file, you need to add your own loop here to handle that. I also changed chop to chomp, since that will only remove line-ends, preventing bugs if the last line in your file doesn't have a line-end on it, and removed $word, since it wasn't being used. This gives us:

while ($list) {
  chomp;
  s/\r//;
  for (my $letter = split(//)) {
	$curlist[ord $letter]++;
  }
}

$letter isn't strictly necessary, but it makes the code more readable than using ord on an implicit $_.

$strid = join(", @curlist);

Another double-quote which should be two single-quotes.

$wordlist{$strid} = $word;

Note that, if $wordlist{$strid} already exists (i.e., another word with the same set of letters in it was already seen), this will replace it, so you'll only get the last word seen with those letters. To keep track of them all, you need to store an array for each $strid:

push @{$wordlist{$strid}}, $word;

Come to think of it, I don't see why you're making a huge array of character counts instead of just using sorting the letters in the word and using that as your ID:

$strid = join('', sort split(//, $word));

while () {

I assume that this is supposed to be looping over some source of input, such as data entered at the keyboard, in which case you would want

while(<STDIN>)

if (exists($wordlist{$strid})) {
print $wordlist{$strid} . "\n";
push @inlist, $wordlist{$strid};

To deal with multiple words on the same key in %wordlist, this needs to become:

  if (exists $wordlist{$strid}) {
	for (@{$wordlist{$strid}}) {
	  print "$_\n";
	}
	push @inlist, @{$wordlist{$strid}};

print "*** " . $wrd . ": not found\n";

Double-quotes interpolate variable values into the string, so you don't need to concatenate the strings with . here, you can just say

print "*** $word : not found\n";

So... Taking all that and doing a little basic cleanup (defining lexical ("my") variables in the smallest possible scope, rearranging things to better reflect logical flow, etc.), we end up with:

#!/usr/bin/perl

use strict;
use warnings;

my $filename = '/wordlist.txt';
my %wordlist;

open(my $list, '<', $filename) or die "Failed to open $filename: $!\n";
while (my $word = <$list>) {
  chomp $word;
  my $strid = join('', sort split(//, $word));
  push @{$wordlist{$strid}}, $word;
  print "$word:";
}
close($list);

while (my $word = <STDIN>) {
  chomp $word;
  my @inlist;
  my $strid = join('', sort split(//, $word));
  if (exists $wordlist{$strid}) {
	for (@{$wordlist{$strid}}) {
	  print "$_\n";
	}
	push @inlist, @{$wordlist{$strid}};
  } else { 
	print "*** $word : not found\n";
	push @inlist, '-';
  }
  print join(',', @inlist);
}

The output is a bit messy (it could really use a few more newlines and you don't want to print every word as it's read in if you're loading in a dictionary of a quarter-million words...) and it doesn't handle multi-word anagrams (e.g., it won't recognize that "a decimal point" = "I'm a dot in place"), but it does work within the limits of what you appeared to be doing with your initially-posted code.

[cloudnyn3]

Well I've revised and made changes based on what you put down, I'm really new to perl, I work well with php but perl is so different =/

This is the revised edition which doesn't work, I"ve googled as many errors as I can and it gets better everytime but theres still a few errors I can't seem to fix =/
I copy and pasted your code and ran it, but it just prints the entire wordlist =/
so I assume thats not what you were wanting me to do.
Right now I'm using this but I'm getting these errors =/

C:\Perl\bin>ana.pl
Use of implicit split to @_ is deprecated at C:\Perl\bin\ana.pl line 17.
Global symbol "$word" requires explicit package name at C:\Perl\bin\ana.pl line
22.
Global symbol "$word" requires explicit package name at C:\Perl\bin\ana.pl line
23.
Global symbol "$word" requires explicit package name at C:\Perl\bin\ana.pl line
36.
Global symbol "$word" requires explicit package name at C:\Perl\bin\ana.pl line
43.
Missing right curly or square bracket at C:\Perl\bin\ana.pl line 47, at end of line

#!/usr/local/bin/perl

use strict;
use warnings;

my $filename = "/wordlist.txt";
my %wordlist;
my @curlist;
my $strid;
my $wrd;
my @inlist;
open(my $list, '<', $filename) or die "Failed to open $filename: $!\n"; {
@curlist = (0) x 508;
while ($list) {
  chomp;
  s/\r//;
  for (my $letter = split(//)) {
	$curlist[ord $letter]++;
  }
}
$strid = join('', @curlist);
push @{$wordlist{$strid}}, $word;
print $word . "\t" . $strid . "\n";

close(LIST);
print "word:";
while(<STDIN>) {
chop;
@inlist = ();
foreach (split(/,/)) {
@curlist = (0) x 508;
$wrd = $_;
foreach (split(//)) {
$curlist[ord]++;}
}
$strid = join('', sort split(//, $word));
  if (exists $wordlist{$strid}) {
	for (@{$wordlist{$strid}}) {
	  print "$_\n";
	}
	push @inlist, @{$wordlist{$strid}};
} else {
print "*** $word : not found\n";
push @inlist, '-';
}

print join(',', @inlist);

I really appreciate you helping me out, I know writing code for a long time gets kinda boring and it looks like you did alot of work =)

[dsherohman]

cloudnyn3, on 20 Aug, 2009 - 10:16 PM, said:

I copy and pasted your code and ran it, but it just prints the entire wordlist =/
so I assume thats not what you were wanting me to do.

It prints the entire wordlist because line 14 of my version (print "$word:" tells it to print each word in the list after adding it. Once it's done reading in the word list, it waits for you to enter something to test against the word list to see whether any anagrams are present. You can comment that line out if you don't want it to spew the entire word list at you (which is what I had to do while testing it, since I used /usr/share/dict/words as my wordlist and I didn't want to wait for it to print all 234937 words).

C:\Perl\bin>ana.pl
Use of implicit split to @_ is deprecated at C:\Perl\bin\ana.pl line 17.

This is actually just a warning, not an error, but it's still something you probably shouldn't be doing. Line 17 is

  for (my $letter = split(//)) {

This line is splitting the contents of $_ into a list which is being implicitly passed through the array @_, which, as the warning says, is deprecated. "perldoc perldiag" tells us:

Use of implicit split to @_ is deprecated
	(D deprecated) It makes a lot of work for the compiler when you
	clobber a subroutine's argument list, so it's better if you assign
	the results of a split() explicitly to an array (or list).

After clobbering @_, this line then assigns that array to the scalar $letter. Being a scalar, $letter can only hold a single value, which will be the number of letters in the original word:

$ perl -e '$foo = split //, "bar"; print "$foo\n";'
3

To do the split and loop over the letters in the word, you would use

for my $letter (my @letters = split(//)) {

to split the word into the array @letters (thus avoiding the warning about splitting into @_) and then loop over @letters, assigning $letter to each in turn.

Global symbol "$word" requires explicit package name at C:\Perl\bin\ana.pl line 22.
Global symbol "$word" requires explicit package name at C:\Perl\bin\ana.pl line 23.
Global symbol "$word" requires explicit package name at C:\Perl\bin\ana.pl line 36.
Global symbol "$word" requires explicit package name at C:\Perl\bin\ana.pl line 43.

In your block of variable declarations at the start of the program, you declared "my $wrd", but not "my $word". Given that $wrd is only used once (in the line "$wrd = $_"), I suspect this is a typo and both appearances of "$wrd" should be changed to "$word".

Missing right curly or square bracket at C:\Perl\bin\ana.pl line 47, at end of line

If I'm following the intended logic correctly, you need to add another } (the "missing right curly bracket") at the end of the program. Your left brace from the line

while(<STDIN>) {

was never closed.

This type of problem is, incidentally, much easier to spot (and to avoid in the first place) if you use a consistent style of indentation (you had several unindented lines) and brace placement (the line "$curlist[ord]++;}" initially threw me off because I missed the "}" hiding at the end of the line).

cloudnyn3, on 20 Aug, 2009 - 10:16 PM, said:

I really appreciate you helping me out, I know writing code for a long time gets kinda boring and it looks like you did alot of work =)

No problem. I'm a big fan of Perl and helping out with these kinds of problems makes a nice change of pace when I want to procrastinate a little on my current coding projects. It's also my way of helping to show the world that, yes, you can write clear, readable, maintainable Perl and it's not even that difficult to do.

[KevinADC]

This also looks wrong:

while ($list) {

$list is the indirect filehandle and should be inside <>:

while (<$list>) {

This is also wrong:

for (my $letter = split(//)) { 

split() returns a list, when you assign the return value of split() to a scalar it returns the number of splits made, not the list of atoms.

It can be written like this:

  for (split(//)) {
	$curlist[ord $_]++;
  }

although that is not a very clear way of writing code.

[chorny_cpan]

Consider reading "Learning Perl" (no earlier than 4th edition) or "Beginning Perl" - they both have good exercises.

Adding "use diagnostics;" will also help you.