OK, now I understand why things are formatted as they are. I
suppose this also explains why you don't indent subroutines :-)
What I do know is I edit in terminal windows that I stretch
out to 50 rows by 110 columns or more, so even with loads of whitespace
I can see a lot of information at once.
Some more comments, now that I've looked at the code in more detail.
Update: more comments added 23-jul-2001
- This first point is pretty anal-retentive, so don't take it too seriously.
This gets back to the whitespace argument, but ceteris parabus I don't
believe you lose anything. The point is I find...
my$temp = '/home/dlandgre/perl/reputer/'; # where data files are sav
+ed
my$public_access = ''; # 'yes' disables config, downloads, and exter
+nal program functions
my$bodytag = '<body bgcolor="#aaaaaa" text="#000000">';
my$form_method = 'get'; # get or post, 'get' shows params in url
...much harder to scan than...
my$temp = '/home/dlandgre/perl/reputer/'; # where data file
+s are saved
my$public_access = ''; # 'yes' disables config, downloads, and exter
+nal program functions
my$bodytag = '<body bgcolor="#aaaaaa" text="#000000">';
my$form_method = 'get'; # get or post, 'get' shows params in url
- In a similar vein, there are a number of areas in the code where you
initialise a whole slew of variables at once....
my($mode,$td,$ta,$rd,$ra,$ca,$cd) = '';
...although this only initialises the first variable in the list. (I think this
was brought up in the CB, but I didn't pay close attention). Anyway, my
first cut at getting it right was...
my($mode,$td,$ta,$rd,$ra,$ca,$cd) = ('','','','','','','');
I ended up, however, doing something else (when I wound up being off by one).
Where you may agree to disagree with me (given that it chews up a lot
more space, I much prefer reading...
my $mode = '';
my $td = '';
my $ta = '';
... it just stands out more clearly.
- Using unless to introduce large conditional blocks. I don't really
like that. Rather than...
unless($i{'n'} eq 'logout'){
if($ins > 1){$s='s'}
&begin_html('fu');
print qq~$bq $bq $bq
...I prefer to reverse the comparison operator and say if ($i{'n'} ne 'logout').
I tend to use unless only in the following scenario...
$var += 2 unless $condition;
... i.e. trivial conditions.
- Similarly if($ins > 1){$s='s'} is better written as $s='s' if $ins > 1
and as an added benefit you avoid the cost of building up and tearing down a code block.
- You are using Data::Dumper to serialise the data between
invocations. Are you aware of Storable? It's much faster. I
wrote a benchmark to load the reputer.dat file, increment the reputation
of a node and then store it out again. I had to hack the data structure
a bit, because Storable wants to serialise one reference only per file.
#! /usr/bin/perl -w
use strict;
use Data::Dumper;
$Data::Dumper::Indent = 0;
use vars qw/$xpdat1 $dat1 $makedat/;
my $datafile = shift or
die "No data file specified on command line.\n";
my $today = localtime(time);
my $root;
eval "require '$datafile'" or die "bad eval: $@\n";
$root->{DATA} = $dat1;
$root->{XP} = $xpdat1;
$root->{MAKE} = $makedat;
open DAT, ">$datafile" or die "Cannot open $datafile for output: $!\
+n";
print DAT Data::Dumper->Dump( [$root], ['$root'] );
close DAT;
Note that the...
print DAT Data::Dumper->Dump( [$root], ['$root'] );
...construct saves out the data structure with the correct name, as opposed to
using...
$var = 'root';
$Data::Dumper::Varname = "$var";
print DAT Dumper($root);
...with the funny $xpdat1 $xpdata dot-and-carry-one method. Once the
data is encoded in this manner, we can then begin to compare Data::Dumper
versus Storable.
#! /usr/bin/perl -w
use strict;
use Data::Dumper;
use Storable;
use Benchmark;
use vars qw/$root $root1/;
my $ddi = 0; # data dumper intent level, 0 = smallest files
my $var;
my $datafile = shift or die "No data file specified on command line.
+\n";
my $today = localtime(time);
my $storefile = $datafile . '.store';
eval "require '$datafile'" or die "bad eval: $@\n";
$root = $root1;
store $root, $storefile;
my $debug = 0;
sub via_eval_dump {
eval "require '$datafile'" or die "bad eval: $@\n";
open DAT, ">$datafile" or die "Cannot open $datafile for output: $
+!\n";
$root = $root1;
$root->{DATA}{NODE}[45]{'reputation'} += 1;
$debug and print "$root->{DATA}{NODE}[45]{'content'} $root->{DATA}
+{NODE}[45]{'reputation'} \n";
$Data::Dumper::Indent = $ddi;
$var = 'root';
$Data::Dumper::Varname = "$var";
print DAT Dumper($root);
close DAT;
}
sub via_retrieve_store {
$root = retrieve $storefile;
$root->{DATA}{NODE}[45]{'reputation'} += 1;
$debug and print "$root->{DATA}{NODE}[45]{'content'} $root->{DATA}
+{NODE}[45]{'reputation'} \n";
store $root, $storefile;
}
timethese shift(), {
'eval & dump' => \&via_eval_dump,
'retrieve & store' => \&via_retrieve_store,
}
Bear in mind though, that the datafile produced by Storable
is not human-readable. You have to write companion scripts to repackage it via Data::Dumper if
you need to eyeball it afterwards. The results are very eloquent...
% ./benchstore reputer.dat 5000
Benchmark: timing 5000 iterations of eval & dump, retrieve & store...
eval & dump: 536 wallclock secs (533.99 usr + 1.51 sys = 535.50 CPU)
retrieve & store: 29 wallclock secs (26.67 usr + 2.71 sys = 29.38 CPU)
- Once again, are you interested in diff patches? Should I post them to the
reputer code node?
Hmm, I guess that's enough rambling for one day. This is a darn useful
piece of code and I'm surprised more people aren't playing
with it (and making suggestions).
update 23-jul-2001
I rewrote the graph routine. All I really wanted to do was to put a spacer between the
highlighted number and the histogram in the next column so that they don't run on into
each other. I wound up coalescing other bits and pieces to make things fit more cleanly
in the code.
sub graph
{ # calculate and display the graphs
my($hm1,$hm2,$hm3,$bdr,$cs,$spacer,$mode,$descr);
if(!$i{histmode} or $i{histmode} == 1){
$hm1 = ' checked';
$spacer = '<td width="1"></td>';
$cs = 4;
$bdr = 0;
$descr = qq~<b>mode one</b><br><small>
bar height = fixed<br>
bar width = number of nodes at that rep<br></small></td></tr>~
}
elsif($i{histmode} == 2){
$hm2 = ' checked';
$spacer = '';
$cs = 3;
$bdr = 1;
$descr = qq~<b>mode two</b><br><small>
bar height = rep<br>
bar width = number of nodes at that rep<br></small></td></tr>~
}
elsif($i{histmode} == 3){
$hm3 = ' checked';
$spacer = '<td width="1"></td>';
$cs = 4;
$bdr = 0;
$descr = qq~<b>mode three</b><br><small>
bar height = number of nodes at that rep<br>
bar width = rep<br></small></td></tr>~
}
print qq~<p><table border="$bdr" cellpadding="0" cellspacing="0" width
+="100%">
<tr><th align="left" colspan="$cs" valign="top">
<table border="0" cellpadding="0" cellspacing="0" width="100%" bgcolor
+="#cfcfcf">
<tr><th align="left">
<h2>Number of nodes by reputation
<font size="-1"><br>average rep and maximum num highlighted</font></h2
+></th>
<td><table align="right" border="0" cellpadding="3" cellspacing="0"><t
+r><td>$descr
<tr><form method="$form_method"><td align="right" valign="bottom">
1<input type="radio" name="histmode" value="1"$hm1>
2<input type="radio" name="histmode" value="2"$hm2>
3<input type="radio" name="histmode" value="3"$hm3>
<input type="hidden" name="n" value="graph">
<input type="submit" value="mode"></td></form></tr></table>
</td></tr></table></td></tr>~;
my@high = sort {$b <=> $a} values %rep_freq;
my@hig = sort {$b <=> $a} keys %rep_freq;
my$mult = sprintf "%d", (600/$high[0]); # normalize bar width to scale
+ (highest num = 600 pixels)
my$mul = sprintf "%d", (600/$hig[0]); # normalize bar width to scale
+(highest num = 600 pixels)
print qq~<tr bgcolor="#b0b0b0"><td><b>$nb rep $nb</td><td><b>$nb num $
+nb</td><td> </td></tr>~;
my($ar,$fc,$arf,$fcf) = '';
for(sort {$b <=> $a} keys %rep_freq){
my$w = ($rep_freq{$_}*$mult);
my$h = 5;
if($i{'histmode'}){
if($i{'histmode'} == 2){ $h = $_ || 1 }
elsif($i{'histmode'} == 3){ $w = ($_*$mul); $h = $rep_freq{$_}
+ }
}
if($_ == $avgrep){ # highlight avg rep
$ar = 'bgcolor="#880066"';
$fc = '<font color="white">'
}
else{$ar=''; $fc='';}
if($rep_freq{$_} == $high[0]){ # highlight frequency high
$arf = 'bgcolor="#880066"';
$fcf = '<font color="white">'
}
else{$arf=''; $fcf=''}
print qq~<tr><td $ar align="right">$fc $_ $nb</td><td $arf align="
+right">$fcf $rep_freq{$_} $nb</td>
$spacer<td><table border="0" cellpadding="0" cellspacing="0">
<tr><td bgcolor="#880066"><img src="$uri?n=gif" width="$w" height="$h"
+ border="0"></td></tr></table>
</td></tr>~
}
print qq~<tr bgcolor="#b0b0b0">
<td><b>$nb rep $nb</td><td><b>$nb num $nb</td><td> </td></tr></ta
+ble><p>~;
}
-- g r i n d e r
-
Are you posting in the right place? Check out Where do I post X? to know for sure.
-
Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
<code> <a> <b> <big>
<blockquote> <br /> <dd>
<dl> <dt> <em> <font>
<h1> <h2> <h3> <h4>
<h5> <h6> <hr /> <i>
<li> <nbsp> <ol> <p>
<small> <strike> <strong>
<sub> <sup> <table>
<td> <th> <tr> <tt>
<u> <ul>
-
Snippets of code should be wrapped in
<code> tags not
<pre> tags. In fact, <pre>
tags should generally be avoided. If they must
be used, extreme care should be
taken to ensure that their contents do not
have long lines (<70 chars), in order to prevent
horizontal scrolling (and possible janitor
intervention).
-
Want more info? How to link
or How to display code and escape characters
are good places to start.
|
|