Beefy Boxes and Bandwidth Generously Provided by pair Networks
Just another Perl shrine
 
PerlMonks  

Comment on

( #3333=superdoc: print w/ replies, xml ) Need Help??

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

In reply to more comments about reputer by grinder
in thread reputer reply by epoptai

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post; it's "PerlMonks-approved HTML":



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.
  • Log In?
    Username:
    Password:

    What's my password?
    Create A New User
    Chatterbox?
    and the web crawler heard nothing...

    How do I use this? | Other CB clients
    Other Users?
    Others browsing the Monastery: (9)
    As of 2015-07-08 03:35 GMT
    Sections?
    Information?
    Find Nodes?
    Leftovers?
      Voting Booth?

      The top three priorities of my open tasks are (in descending order of likelihood to be worked on) ...









      Results (94 votes), past polls