Beefy Boxes and Bandwidth Generously Provided by pair Networks
P is for Practical
 
PerlMonks  

more comments about reputer

by grinder (Bishop)
on May 22, 2001 at 17:34 UTC ( #82216=note: print w/ replies, xml ) Need Help??


in reply to reputer reply

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">&nbsp;&nbsp; <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>&nbsp;</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>&nbsp;</td></tr></ta +ble><p>~; }

--
g r i n d e r


Comment on more comments about reputer
Select or Download Code

Log In?
Username:
Password:

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

How do I use this? | Other CB clients
Other Users?
Others browsing the Monastery: (13)
As of 2014-09-18 14:25 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    How do you remember the number of days in each month?











    Results (116 votes), past polls