in reply to From test harness to CPAN

Taking things in backwards order:

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.

Handle errors using exceptions (die and eval). That makes dealing with errors in an appropriate place much easier for your code and for the caller's code.

With OO code helper functions are just something provided by the object.

A partial implementation of your code as OO might look like:

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); } } }

so the module might be used like:

use Sport::Analytics::SRS; eval { my $analysis = Sport::Analytics::SRS->new (); $analysis->loadData (); $analysis->fullMatrix (); ... } or do { die "Analysis failed: $@\n"; };
True laziness is hard work

Replies are listed 'Best First'.
Re^2: From test harness to CPAN
by dwm042 (Priest) on Apr 17, 2011 at 13:34 UTC
    Grandfather,

    There are some clever ideas in your post and I buy the idea that an object would make things easier. In a PDL implementation there will be a temptation to add features, because once you do the work of dataLoad, you'll want to have access to "more stuff". That's item one.

    Item two is I'm testing your idea and the moment I start using the "pdl" command, the Perl interpreter doesn't appear to take well to the 'load it if we need it' idea.

    I'm thinking two objects, a simpler one for non-PDL use and a more complicated one (that could do more kinds of rankings) for PDL use. But those are my current thoughts.

    Using Sport as a base (the singular word as opposed to plural) is interesting as then people could write analysis objects for a particular sport.. Sport::Cricket or Sport::Baseball, etc.

    David.

      Most likely the 'load it if we need it' failed for a second object because PDL was already loaded. I was thinking more in terms of "use it if it's available" in any case. The following code implements that:

      use strict; use warnings; package TestPDL; my $PDLLoaded = eval { require 'PDL.pm'; PDL->import (); 1; }; sub new { my ($class, %params) = @_; $params{havePDL} = $PDLLoaded; return bless \%params, $class; } package main; my $obj = TestPDL->new (); my $obj2 = TestPDL->new (); print $obj2->{havePDL} ? "Have PDL available" : "PDL not present or object create failed";

      I'd write one basic object that does common stuff then derive from that to specialise for additional capabilities. Following that thought, I'd arrange the name space as Sport::Analytics::SRS for the basic module then Sport::Analytics::SRS::Cricket for the Cricket specialisation.

      This technique allows a PDL only implementation for tricky or computationally expensive stuff with a pure Perl implementation provided either in the module or in a derived class at some point in the future if there is a need for it.

      True laziness is hard work