Perl OOP: Is it a good idea to outsource methods in your own modules

Viewed 111

I am considering moving the individual methods of a module into separate modules in order to obtain more manageable files. I wrote a little test for this:

a.pl:

#!/usr/bin/perl

use 5.028;
use warnings;
use utf8;
use open ':std', ':encoding(UTF-8)';
use Readonly;
use English qw(-no_match_vars);
use Benchmark qw(:all);
use A;


our $VERSION = 1;

Readonly::Scalar my $COUNT => 10_000_000;

warn $A::VERSION;
warn $A::Login2::VERSION;
my $a = A->new;
warn $a;
$a->login(1);
$a->login2(1);

cmpthese($COUNT, {
  login  => sub{$a->login},
  login2 => sub{$a->login2}
});

A.pm:

package A;
use 5.028;
use warnings;
use utf8;
use open ':std', ':encoding(UTF-8)';
use Readonly;
use English qw(-no_match_vars);
use A::Login2 'login2';


our $VERSION = 1;

sub new {
  my ($class, $p) = @_;
  my $this = {};
  bless $this, $class;
  return $this;
}

sub login {
  my ($this, $dump) = @_;

  if ($dump) {
    warn "$this: login";
    $this->test;
  }

  return;
}

sub test {
  my ($this) = @_;
  warn "$this: test";
  return;
}

1;

A/Login2.pm:

package A::Login2;
use 5.028;
use warnings;
use utf8;
use open ':std', ':encoding(UTF-8)';
use Readonly;
use English qw(-no_match_vars);
use base 'Exporter';
our @EXPORT_OK = qw(login2);


our $VERSION = 1.1;

sub login2 {
  my ($this, $dump) = @_;

  if ($dump) {
    warn "$this: login2";
    $this->test;
  }

  return;
}

1;

The output from ./a.pl is:

1 at ./a.pl line 18.
1.1 at ./a.pl line 19.
A=HASH(0x5581d48f3470) at ./a.pl line 21.
A=HASH(0x5581d48f3470): login at A.pm line 25.
A=HASH(0x5581d48f3470): test at A.pm line 34.
A=HASH(0x5581d48f3470): login2 at A/Login2.pm line 18.
A=HASH(0x5581d48f3470): test at A.pm line 34.
            Rate  login login2
login  5847953/s     --    -6%
login2 6250000/s     7%     --

I would have thought that login was faster than login2.

  1. Why login2 is faster than login?
  2. Is it a good Idea to put each method in its own module?
    1. Is there a better way?

I'm looking forward for comments.

2 Answers

This is an open-ended question on design, but I'd offer some specific comments.

Firstly, it's a commendable want to split up unwieldy files for management ease and readability, and this is in general a good idea. We use libraries in any sizable code we write -- so right there, the overall code is split across different units.

But such division is based on functionality, where behaviors (functions) are naturally grouped into packages. Splitting for size may result in an awkward codebase; it may matter what goes together, updating can get tricky (buggy?), etc. This can actually hinder the overall managability.

If a module feels too large it may well be that there is too much functionality bundled together and that the codebase should be in several different modules. There is no simple rule for assessing this; designing libraries isn't easy. It may be helpful to think whether it makes sense splitting off a group of functions so that they have their own namespace? †

However, the posted example has another issue: it's a class but where object-oriented mechanism is mixed with the basic package import. This is convoluted (how/why does an object get passed to a function defined in a file which isn't a class?), and I wouldn't recommend doing that.

Can a well designed class be too large to nicely fit in a file? Probably I guess, even though I haven't seen such a case. Usually, when a codebase ends up broken across multiple compilation units in Perl it is because of functionality -- it is more fittting to have multiple classes.

But if somehow mere size ends up being the problem, a reasonable approach may be to have multiple files each being the same package, clearly documented.

A.pm

package A;

use warnings;
use strict;
use feature 'say';

# =======================================================
# NOTE: Class definition is given in multiple parts/files
# =======================================================

use A_part1;
use A_part2;

sub new { ... } 

# perhaps more methods in this file
1;

A_part1.pm

package A;

# warnings, strict, pragmas, etc

sub func1 { my ($self, @args) = @_; ... };

...    

1;

and similarly for A_part2.pm etc. Then this is used as usual

use A;

my $obj = A->new( ... );

$obj->func1(...); 

Note that this breaks the rule (a convention) about the relation between the file name and the package name (A_part1.pm vs package A;); for one, PerlCritic will complain.‡ However, it is done deliberately here and I wouldn't be concerned about that.

But I'd consider this to be actually needed only very rarely. I'd rather expect that if a library seems too large it is probably taking on too much and should be redesigned into multiple classes.


† But if there is indeed simply too many functions, which do belong in the same library, once the file is broken up consider using require to bring those files together.

‡ Perl::Critic::Policy::Modules::RequireFilenameMatchesPackage

tl;dr: Separate groups of methods into manageable chunks. Don't put every method into its own file, but don't put a million methods in one file. How you split that up depends on the task, how the methods relate to each other, and your tolerance for editing it all. Performance is not the key issue: maintainability is. Worry about performance when it becomes an issue.


You see a differences of about 6% in your test, but there are a few things to consider:

  • The uncertainty in Benchmark is estimated to be in about that range. See, for instance, Steffen Mueller's writings about benchmarking and his Dumbbench module.

  • You benchmark shows something happening just under 6 million times a second versus something happening just over 6 million times a second. That's really, really fast in both cases. If you added a control, like a plain sub { 1 }, I think you'd find that the mere act of testing is the most significant factor in your results. Consider that you also want to test all of the other cases, including calling the method from its source package without the import, calling everything as plain subroutines (without method dispatch), and various other ways. You need to tease out what's actually important by isolating different factors.

  • I have various talks about benchmarking where the code isn't doing what we think it is doing, as in Wasting time thinking about wasted time. Since you run so many iterations per second, I'm guessing that you aren't actually measuring anything real. Once Perl knows how to resolve a method, for example, it doesn't need to do that work again. Everything you are wondering about this problem has already happened before your benchmark starts.

  • You have a lot of extra code (Readonly, English, etc). Unless those things are part of what you are trying to measure, get rid of them. I'm not even sure they are doing what you think they are doing. You also don't need to inherit from Exporter (thus screwing around with @ISA). You just want its import routine, which you can import.

Related