note
GrandFather
<p>Taking things in backwards order:</p>
<ul>
<li>5/ What did you search for when you checked CPAN for prior art?
<p>Ask yourself what someone interested in this stuff is most likely to search for and chose a name space and a name that is a good fit for that search. Also make sure that the key words appear in the POD.</p>
<p>So far as I can find, at present Sport doesn't feature on CPAN (seems a large omission), nor does any sport related analytics.</p>
</li>
<li>4/ Absolutely use helper functions, but see comments later.</li>
<li>3/ Absolutely use error checking on input, and anywhere else that will catch problems early rather than late. See comments below.</li>
<li>2/ See comments below.</li>
<li>1/ See comments below.</li>
</ul>
<p>After a quick glance at your code there are a few things I'd change. First off, make it OO. That way you don't need to pass back a mess of data that your client is just going to have to pass back in elsewhere. It also makes dealing with PDL/non-PDL handling tidier.</p>
<p>Handle errors using exceptions ([doc://die] and [doc://eval]). That makes dealing with errors in an appropriate place much easier for your code and for the caller's code.</p>
<p>With OO code helper functions are just something provided by the object.</p>
<p>A partial implementation of your code as OO might look like:</p>
<c>
use strict;
use warnings;
package Sport::Analytics::SRS;
return 1; # madule returns true for successfull load
sub new {
my ($class, %params) = @_;
# parameter validation here. die on failure.
# Test for PDL
$params{havePDL} = eval {
require 'PDL';
PDL->import ();
1;
};
return bless \%params, $class;
}
sub srs_full_matrix {
my ($self, %options) = @_;
die "The module PDL must be available for srs_full_matrix use."
if !$self->{havePDL};
$options{epsilon} ||= 0.001;
$options{maxiter} ||= 1000000;
$self->{srs} = $self->{mov}->copy ();
my $oldsrs = $self->{srs}->copy ();
my $delta = 10.0;
my $iter = 0;
while ($delta > $options{epsilon} and $iter < $options{maxiter}) {
my $wt = 1.0 / sumover ($self->{played});
my $prod = $self->{srs} x $self->{played};
my $sos = $wt * $prod;
$self->{srs} = $self->{mov} + $sos;
$delta = max (abs ($self->{srs} - $oldsrs));
$oldsrs = $self->{srs}->copy ();
$iter++;
}
print "iter = $iter\n" if $options{debug};
print "epsilon = $options{epsilon}\n" if $options{debug};
printf "delta = %7.4f\n", $delta if $options{debug};
my $offset = sum ($self->{srs});
$self->{srs} -= ($offset / $self->{mov}->nelem);
return $self->{srs}->slice (":,(0)");
}
sub loadData {
my ($self, $games) = @_;
for (@$games) {
my ($visitor, $visit_score, $home_team, $home_score) = split "\,", $_;
my $diff = $home_score - $visit_score;
$self->{team}{$visitor}{games_played}++;
$self->{team}{$home_team}{games_played}++;
$self->{team}{$visitor}{points} -= $diff;
$self->{team}{$home_team}{points} += $diff;
push @{$self->{team}{$visitor}{played}}, $home_team;
push @{$self->{team}{$home_team}{played}}, $visitor;
}
my $total_team = scalar keys %{$self->{team}};
$self->{played} = zeroes ($total_team, $total_team);
$self->{mov} = zeroes ($total_team);
my $ii = 0;
for (sort keys %{$self->{team}}) {
my $team_diff =
$self->{team}{$_}{points} / $self->{team}{$_}{games_played};
$self->{team_map}{$_} = $ii;
$self->{mov}->set ($ii, $team_diff);
$ii++;
}
for (keys %{$self->{team}}) {
my $i = $self->{team_map}{$_};
for my $opp (@{$self->{team}{$_}{played}}) {
my $j = $self->{team_map}{$opp};
my $a = $self->{played}->at ($i, $j);
$a++;
$self->{played}->set ($i, $j, $a);
}
}
}
</c>
<p>so the module might be used like:</p>
<c>
use Sport::Analytics::SRS;
eval {
my $analysis = Sport::Analytics::SRS->new ();
$analysis->loadData ();
$analysis->fullMatrix ();
...
} or do {
die "Analysis failed: $@\n";
};
</c>
<div class="pmsig"><div class="pmsig-461912">
True laziness is hard work
</div></div>
899752
899752