I just can't see the error in this code

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

Replies are listed 'Best First'.
Re: I just can't see the error in this code by duff (Parson) on Dec 01, 2004 at 07:08 UTC
Oh my. Where to begin? Hmm. How about here: `while (<SHORTNAMES>) { # loop through all lines of shor +tnames my @array2 = split /:/; # seperate by those ugly colens + created by OSX $finishedusers{$array2[7]} = $array2[0];# Setup keys: "Long Name" + => "shortname" }` [download] What's with all of the commentary? Why not make your code self-documenting? To wit: `open SHORTNAMES, "< $usershortfile" or die "$!"; while (<SHORTNAMES>) { my ($longname,$shortname) = (split /:/)[7,0]; $finishedusers{$longname} = $shortname; } close SHORTNAMES;` [download] Notice how I use descriptive variable names? Also, I closed the file when I was finished mucking with it. Good practice that. And you should also open a file quite near where you intend on using it. The `USERNUMS` file is open for quite a while before you ever do anything with it. Skipping down ... surely this can't be right? `my %final; # Hash %final while (($fullname, $uid) = each(%final)) { # Loop through hash` [download] You're creating an empty hash then iterating over it. That's going to be one short loop! :-) And even if your loop did work, it's doing too much work because you're not taking advantage of the nature of hashes but rather iterating over another hash to find the keys that match in the first one. The canonical way to do this is simply use the key from one hash as the key into the other: `while (($fullname, $uid) = each(%lookup)) { # corrected to use %l +ookup hash instead. next unless exists $finishedusers{$fullname}; system ("cp '$oldserv/$uid/Documents/*' '$newserv/$finishedusers{$f +ullname}/Documents/'"); print "Copying Files for $fullname"; }` [download] I think I'll stop there as I'm tired now. duff	[reply] [d/l] [select]
Re: I just can't see the error in this code by castaway (Parson) on Dec 01, 2004 at 07:51 UTC
Just in case you're wondering why your post is getting downvoted, you might want to go and read How do I post a question effectively?. Especially the point(s) that talk about "Saying what went wrong/what the error message is", "Posting sample data" etc. We'd like to be able to be mind readers, but unfortunately we're not. C.	[reply]
Re: I just can't see the error in this code by trammell (Priest) on Dec 01, 2004 at 14:24 UTC
You forgot: `use strict; use warnings;` [download] As an aside, you also might be over-commenting, e.g. `my %final; # Hash %final` [download]	[reply] [d/l] [select]
Re: I just can't see the error in this code by htoug (Deacon) on Dec 01, 2004 at 06:29 UTC
Your code does - what? And you expect it to do ...? Given input ...? Give examples please Without some information it is nearly impossible to find any errors.	[reply]
Re^2: I just can't see the error in this code by duff (Parson) on Dec 01, 2004 at 07:10 UTC
Au contraire, I think the problem is that there are too many errors to find while the OP is only interested in one particular error. How to know which though? duff	[reply]
Re: I just can't see the error in this code by thinker (Parson) on Dec 01, 2004 at 10:41 UTC
Hi gsr, As one of our more esteemed monks has pointed out here Perhaps your veeblefitzer is clogged. :-) Hope this helps thinker	[reply]
Re: I just can't see the error in this code by fglock (Vicar) on Dec 01, 2004 at 13:47 UTC
Your use of `%finishedusers` is wrong. You set it with: `$finishedusers{$array2[7]} = $array2[0]; # Setup keys: "Long Name" => "shortname"` Then you reverse it, then you read it with: `while (($fullname2, $short) = each(%finishedusers))` which should really use "short, full" order instead.	[reply] [d/l] [select]
Re: I just can't see the error in this code by gsr (Novice) on Dec 01, 2004 at 20:18 UTC
My apologies for using incorrect syntax in my question. I thought I was responding to a previous thread, not creating a new one (thus, the lack sufficient information, I assumed it was already there). Anyway, I rewritten the code, to make it a bit smaller. My situation is this: My school used to have their student/faculty login system setup, so each user loged in with their full name "First Last". This 'full name' was actually a psydonym for the real UNIX username, which was u character, followed by 3-5 addititonal digits (ie u881). We have since upgraded to a new server, and while doing so, decided to change the real unix usernames, from numbers, to a short version of the user name Gabriel Rigg, u881 becomes Gabriel Rigg, grigg I have two files in my posession: 1) a file containing old usernumbers, and full names: -------------------------usrbynum.txt--------------------- `u880 name: Gabe Girard u881 name: Gabriel Rigg u882 name: Genna Bourget ... for about 263 of them` [download] ----------------------------EOF---------------------------- and 2) a file containing new shortnames, and full names -------------------------srnames.txt----------------------- `fkasierderrick::1333:20::::Freya Kasier Derrick grigg::1460:20::::Gabriel Rigg omeara::1025:20::::Gary O'Meara` [download] ---------------------------EOF----------------------------- There are a few issues to deal with. First, not everyone in short/full is in num/full, but I can deal with that later. However, here is my code: #!/usr/bin/perl $oldserv = "./testing"; $newserv = "./testing2"; $usernumfile = "./usrbynum.txt"; $usershortfile = "./srnames.txt"; open USERNUMS, "< $usernumfile" or die "$!"; open SHORTNAMES, "< $usershortfile" or die "$!";# open file with user while (<SHORTNAMES>) { # loop through all lines of shor +tnames my ($longname,$shortname) = (split /:/)[7,0]; # seperat +e by those ugly colen $finishedusers{$longname} = $shortname; chomp %finishedusers; } my %lookup; # Hash %lookup while(<USERNUMS>) { # loop through file with user numbers ($uid) = /(u\d{3,5})/; # User ID find yes ($longname2) = /([A-Z]\w+)/; chomp $longname2; $lookup{$longname2} = $uid; # Setup hash %lookup with keys " +Long Name"=> "usernum" } while (($longname2, $uid) = each(%lookup)) { next unless exists $finishedusers{$longname2}; system ("cp '$oldserv/$uid/Documents/*' '$newserv/$finishedusers{$ +longname2}/Documents/'"); print "Copying Files for $finishedusers{longname2}"; } close USERNUMS; close SHORTNAMES; [download] To test whether or not the hashes of the different files were working, I commented most of the code out, and told it to to print to the screen. For example: `while (<SHORTNAMES>) { # loop through all lines of shor +tnames my ($longname,$shortname) = (split /:/)[7,0]; # seperat +e by those ugly colen $finishedusers{$longname} = $shortname; chomp %finishedusers; } while (($templong,$tempshort) = each(%finishedusers)) { print "long: $templong \n short: $tempshort \n\n }` [download] For both of the files and their while() statements used to extract data for the hashes, they printed the exact correct statements (even the newlines added up, so there were none because of the chomp). So, I know that, individually at least, these statements work. The final few lines of the code is supposed to match shortnames to old usernumbers, via a matching long name (since its the shared constant between the files), and move all the files from the old usernumber/Directory to the short/Directory. I added 'use strict; use warnings;', and got the following output: Global symbol "$oldserv" requires explicit package name at doctrans2.p +l line 6. Global symbol "$newserv" requires explicit package name at doctrans2.p +l line 7. Global symbol "$usernumfile" requires explicit package name at doctran +s2.pl line 8. Global symbol "$usershortfile" requires explicit package name at doctr +ans2.pl line 9. Global symbol "$usernumfile" requires explicit package name at doctran +s2.pl line 11. Global symbol "$usershortfile" requires explicit package name at doctr +ans2.pl line 12. Global symbol "%finishedusers" requires explicit package name at doctr +ans2.pl line 15. Global symbol "%finishedusers" requires explicit package name at doctr +ans2.pl line 16. Global symbol "$uid" requires explicit package name at doctrans2.pl li +ne 21. Global symbol "$longname2" requires explicit package name at doctrans2 +.pl line 22. Global symbol "$longname2" requires explicit package name at doctrans2 +.pl line 23. Global symbol "$longname2" requires explicit package name at doctrans2 +.pl line 24. Global symbol "$uid" requires explicit package name at doctrans2.pl li +ne 24. Global symbol "$longname2" requires explicit package name at doctrans2 +.pl line 27. Global symbol "$uid" requires explicit package name at doctrans2.pl li +ne 27. Global symbol "%finishedusers" requires explicit package name at doctr +ans2.pl line 28. Global symbol "$longname2" requires explicit package name at doctrans2 +.pl line 28. Global symbol "$oldserv" requires explicit package name at doctrans2.p +l line 29. Global symbol "$uid" requires explicit package name at doctrans2.pl li +ne 29. Global symbol "$newserv" requires explicit package name at doctrans2.p +l line 29. Global symbol "%finishedusers" requires explicit package name at doctr +ans2.pl line 29. Global symbol "$longname2" requires explicit package name at doctrans2 +.pl line 29. Global symbol "%finishedusers" requires explicit package name at doctr +ans2.pl line 30. Execution of doctrans2.pl aborted due to compilation errors. [download] However, without use strict and use warnings, the code execute with no errors, but no results. Expected result was that a file ./testing/u881/Documents/test.txt would be moved to ./testing2/grigg/Documents/ . However, it does not get moved (obviously). Any suggestions?	[reply] [d/l] [select]


good chemistry is complicated, and a little bit messy -LW
	PerlMonks