Beefy Boxes and Bandwidth Generously Provided by pair Networks
Think about Loose Coupling
 
PerlMonks  

Re: Stupid, yet simple, sort question

by jeroenes (Priest)
on Oct 24, 2001 at 21:32 UTC ( [id://121205]=note: print w/replies, xml ) Need Help??


in reply to Stupid, yet simple, sort question

A few style remarks:
  1. try 'use strict' for a change.
  2. try to avoid reeling in outside info in your namespace... use nested structures instead. perlref, perldsc, perllol to name a few. A link to perlreftut can be found in my homenode.
  3. read up on sort and perlop. Use these kind of constructs to sort nested structures
    @sorted_keys = sort { $HoH{$b}{'number'} <=> $HoH{$a}->{'number'} } ke +ys %HoH;
  4. The require 'config.txt' looks questionable, to say the least. Try to use a real config format, like AppConfig.
  5. Try to minimalize the use of reserved words (see perlfunc ). They are very confusing..
So, your code:
open(FILE,$log); flock (FILE,3); @users=<FILE>; close(FILE); foreach $lines (@users){ chop($lines); ($ips,$times,$locationold)=split('×',$lines ); $timeoff=$time-$times; push @locations,$locationold if (!$exists{$locationold}); $exists{$locationold}=1; ${$locationold}{online}++; foreach $loc (@locations){ if(${$loc}{online} < "2") { ${$loc}{users} = "user"; } else { ${$loc}{users} = "users"; } } foreach $loc (@locations){ print "${$loc}{online} ${$loc}{users} $loc\n"; }
Could be written as:
use strict; use warnings; use CGI::Carp qw/fatalsToBrowser/; open(FILE,$log) or die "Couldn't open $log: $!"; flock (FILE,3) or die "Couldn't lock $log: $!"; my @users=<FILE>; close(FILE); chomp @users; my %locations; foreach my $lines (@users){ my ($ips,$times,$location)=split('×',$lines); $timeoff=$time-$times; if ( not exists $locations{$location}) { $locations{$location}={online => 1}; } else { $locations{$location}->{'online'}++; } } $location{$_}->{'usertext'}=$location{$_}->{'online'}==1? 'user' : 'us +ers' for keys %locations; foreach my $loc (sort { $b->{'online'} <=> $a->{'online'} }keys %locat +ions){ my $href = $locations{$loc}; print $href->{'online'}." ".$href->{'usertext'}."$loc\n"; }
To start with. I have added: Hashes of hashes, strict compliance (sort of), die statement as open check, fatalstobrowser, and more things.... it's only a start. Code still unchecked, may contain typos etc.

Hope this gets you on your way....

Jeroen
"We are not alone"(FZ)

Updated... still more to come, I guess.

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://121205]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others drinking their drinks and smoking their pipes about the Monastery: (4)
As of 2024-04-25 13:43 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found