Hello HipDeep, and welcome to the Monastery!

I've also seen quite a few references to the concept that you should never access data structures directly, that you should always use a method. But I'm not sure that would apply directly in this case.

That is quite correct, and I don’t see why it wouldn’t apply in this case. Here is how I would refactor the modules:

package Monks::Data;

use strict;
use warnings;
use parent 'Monks';

BEGIN
{
    use Exporter();
    our $VERSION = 20160808.00;
    our @ISA     = qw(Exporter);
}

sub version
{
    $Monks::Data::VERSION;
}

{
    my  @fluffy;

    BEGIN
    {
        @fluffy = qw( Rabbits Minks Cats );
    }

    sub is_fluffy
    {
        my $animal = shift;

        for (@fluffy)
        {
            return 'Yes' if $animal eq $_;
        }

        return 'No';
    }
}

{
    my  %textures;

    BEGIN
    {
        %textures = 
        (
            Rabbits => 'soft',
            Minks   => 'supersoft',
            Cats    => 'coarse',
        );
    }

    sub get_texture
    {
        my $animal  =  shift;
        my $texture = 'unknown';
           $texture = $textures{$animal} if exists $textures{$animal};

        return $texture;
    }
}

1;
[download]

package Monks;

use strict;
use warnings;
use Monks::Data;

BEGIN
{
    use Exporter;
    our $VERSION = 0.01;
    our @ISA     = qw(Exporter);
}

sub version
{
    $Monks::VERSION
}

sub new
{
    my $class = shift;

    my $self = { @_ };
    bless $self, $class;

    return $self;
};

sub check_fluffy
{
    return Monks::Data::is_fluffy($_[1]);
}

sub find_fur_texture
{
    return Monks::Data::get_texture($_[1]);
}

1;
[download]

The value of this indirection is that it allows the implementation of package Monk::Data to be changed at a later date without impacting the code of its client(s). This is a general OO rule: keep the implementation separate from the public interface, and keep it private.

There are also some situations where I need to access the same type of data that is hard-coded into the main module. The common answer for that seems to be to copy that data into the object when it's created, but that seems inefficient in my case because I have some pretty large structures defined to handle a variety of cases where any individual object is only going to need a sliver of that data.

I don’t understand this. Can you elaborate? — preferably with example code.

Hope that helps,

Thanks, that does help. I had something similar worked up for the constants, but missed the idea of having the function that accesses the data in the subclass, instead of in the parent. So I can work with that. One question, what's the value of the methods in the parent like find_fur_texture, vs. simply calling the method from the child directly? Also, one of the things that concerned me when reading the warnings about inheritance was the idea of calling the methods by name (ala, Monks::Data::get_texture) as opposed to something more abstract. But maybe I'm missing something? And finally I am definitely keeping the goal of keeping the public interface and the internals separate. :)

In regards to the last bit, one example would be a 2d hash with a bunch of compiled regular expressions that I'm using to parse the data that the module deals with. So something like this:

if (not defined $Monks::Parsers{ $self->{thing} }) {
    $self->{parser} = 'Generic';
}
[download]

And:

foreach my $key (keys %{ $Monks::Parsers{ $self->{parser} } }) {
    dostuff;
}
[download]

Does that make sense?

One question, what's the value of the methods in the parent like find_fur_texture, vs. simply calling the method from the child directly?

Think in terms of a client/server architecture: a class (or an object instantiated from that class) is a server, and code which calls its (class or object) methods is a client. A server’s interface should publicly expose only the services it offers. In this case, find_fur_texture is a service (class method) offered by package Monk; the fact that it happens to be implemented inside that class by calling a class method in package Monk::Data is the sort of implementation detail that should remain private to the Monk class.

Put another way: the client code’s access to the services it requires should be kept as simple as possible. Thus, it should use Monk; but it should not have to use Monk::Data.

As Anonymous Monk and kcott have pointed out, there is no need (in the code shown) for package Monk::Data to use parent 'Monk'; — as kcott says, that looks like an OO design flaw. But I agree with kcott in seeing no problem with package Monk useing Monk::Data — that’s simply a part of Monk’s implementation.¹

Also, one of the things that concerned me when reading the warnings about inheritance was the idea of calling the methods by name (ala, Monks::Data::get_texture) as opposed to something more abstract.

I don’t see a problem here. As stated above, clients of the Monk class shouldn’t need to know about the Monk::Data class at all; but clients of Monk::Data have to know the names of its methods, otherwise how can they call them?

In regards to the last bit, one example would be a 2d hash with a bunch of compiled regular expressions that I'm using to parse the data that the module deals with.

This doesn’t sound like a large data structure, only a collection of individual regexen. However, if you do have a data structure large enough to make the costs of copying prohibitive,² that probably indicates that the data structure should itself be encapsulated into its own class. Then Monk::Data can hold and return an instance of that class³ without breaking encapsulation.

¹Yes, this creates a transitive dependency, but that’s really only an issue in compiled languages (see, e.g., the so-called Pimpl idiom in C++). In an interpreted language like Perl, the dependencies to be avoided are those which force client code to be rewritten when the server’s implementation code is changed.
²Beware premature optimisation. Always test your code and profile carefully before expending effort on optimising something that may well be fine as it is.
³In Perl, objects (class instances) are actually blessed references, so passing an object requires little overheard: only a pointer is copied under the hood.

Hope that helps,

Athanasius <°(((>< contra mundum

Iustus alius egestas vitae, eros Piratica,

G'day HipDeep,

Welcome to the Monastery. [At least, as a poster: you appear to have been visiting for some time.]

One of the first things I noticed was that Monks loads Monks::Data and vice versa. While Perl handles this cyclic dependency by only loading modules once, it does highlight a fundamental flaw in your OO design. I'd allow Monks to know about, and load, Monks::Data. On the other hand, Monks::Data does not need to know about Monks; it could, in fact, be used by any number of classes. Using a name that doesn't suggest a subclass of Monks would probably be a good move.

I see no valid use for Exporter in the code you've shown: you export nothing. If, at some stage, you do need to export something, I'd recommend first reading the documentation carefully, especially the "Good Practices" section, and consider "Exporting Without Inheriting from Exporter" (and not modifying @ISA).

With respect to $VERSION, I see no reason for either the BEGIN block or sub version { ... }. I'd also suggest you look at use and version for better $VERSION values.

For constant data, you have a variety of options, which include:

• The constant pragma.
• Using a prototype of '()'; although, do note: "Method calls are not influenced by prototypes ...".
• A subroutine such as: sub { 'constant data' }. [I've used this option in the example code, below.]

If, at some later stage, you use Moose (or similar), consider MooseX::ClassAttribute.

"I've also seen quite a few references to the concept that you should never access data structures directly, that you should always use a method."

Obviously, without seeing these references, I'm guessing here: I suspect this probably refers to using a method (such as $self->get_attr) rather than direct access (like $self->{attr}).

[In the code below, I initially got a bit confused between furry and fluffy, so I've changed everything to furry. Choose whatever you want but consider carefully whether names might introduce problems such as this.]

Here's my suggestion for Monks::Data which, as I said, could be used by any number of classes. It's very basic and only intended to get the general idea across.

package Monks::Data;

use strict;
use warnings;

our $VERSION = '20160808.00';

sub get_furries { [qw{Rabbits Minks Cats}] }

sub get_texture_for { +{qw{Rabbits soft Minks supersoft Cats coarse}} 
+}

1;
[download]

Here's Monks. Like Monks::Data, this is intentionally simplistic.

package Monks;

use strict;
use warnings;

our $VERSION = '0.01';

use Monks::Data;

sub new { 
    my ($class, @args) = @_;
    bless { @args } => $class;
} 

sub is_furry {
    my ($self, $furry) = @_;
    grep { /$furry/ } @{Monks::Data::->get_furries};
}

sub find_fur_texture {
    my ($self, $fur) = @_;
    Monks::Data::->get_texture_for->{$fur};
}

1;
[download]

For an explanation of the '::' following class names (e.g. in Monks::Data::->get_furries), see "perlobj: Invoking Class Methods".

Also note that the object reference ($self) is not used in my examples, but could be.

Here's a script, similar to yours, to use these modules:

#!/usr/bin/env perl -l

use strict;
use warnings;

use Monks;

my $obj = Monks::->new;

for (qw{Cats Alligators Rabbits}) {
    print $_, ': furry? ', $obj->is_furry($_) ? 'Yes' : 'No';
}

print 'Fur texture for Cats: ', $obj->find_fur_texture('Cats');
[download]

Output:

Cats: furry? Yes
Alligators: furry? No
Rabbits: furry? Yes
Fur texture for Cats: coarse
[download]

Ken, thank you so much for your response. You've given me a lot of homework, which I really appreciate. :) I've been using OO code for a long time of course, but I'm new to writing it, and this is exactly the kind of learning experience that I'm after.

Your comments about Exporter are spot on, I have been experimenting with a lot of stuff, and copied and pasted various examples I came across until I had something that worked, then like all good (lazy) programmers I stopped fussing with it. However, I've now got a pretty functional framework for my project, which I now want to start cleaning up and doing things right.

In regards to your comments about the child using the parent, I was reading about 'parent' today, so that's a recent addition. I'm just as happy without it. I get your point about the naming, but the subclass is not really "general purpose" data. I originally thought about a "parallel" package, like Foo_Data, but unless I'm missing something I'm not sure that would solve my data access "issues." I'm also not sure of the propriety of distributing a package that's structured like that, but if it's Ok to do, and it's the right solution, I'm not opposed to doing it.

I'm sort of partial to BEGIN blocks, but I'll read your references. I'd like a way for the user to retrieve the versions, but again, I'll read your references. The date-based version on the data module is a conscious choice, as I'd like an easy reference to when the module was generated.

Your guess is correct regarding $self->get_attr vs. $self->{attr}. I personally find the latter a lot easier to use, but I also don't want to open any more access to the data structures than really necessary. One question occurred to me though, how important is that issue when we're dealing with data that is being parsed out for use by the user? For instance, I have a method that does the actual parsing, and it places the data in a hash. I created methods so that the user can access the data the proper way, but what's to stop them from digging into the code and accessing the hash directly? I did a lot of reading on Perl modules' implied contract with the user that they don't do such things, and if they do, they are on their own if I change the data structure. So if that's the answer, I'm Ok with it.

Sorry about the furry/fluffy thing, I was just trying to have some fun with my sample code. :) Your restructure of the Data module, and the corresponding manner of access are both clear, thanks! And I like simple, it's easier to see what you're doing and how. One question, why the curly braces in the grep in is_furry? Just a style issue? Or does it have a utility in this context that I'm not seeing?

And one more question, and I hope I'm not missing an obvious reason why it's a silly one. :) Is there any benefit to the way that you've set it up (methods that return the data in the Data module, and methods that use the data in the parent) vs. what Athanasius proposed in putting the data and the methods that use the data both in the same package? On one hand I can see an argument for keeping the data module as clean and reusable as possible. OTOH, looking at how I actually use the data, Athanasius' concept is attractive. But I'm still wondering why I couldn't simply call $Monks::Data::->am_i_furry from the main package.

Again, thanks all for your time and effort in helping me learn this stuff. :)

I'd like a way for the user to retrieve the versions

This can be done pretty simply. If we use the modules as kcott has written them, then a script could access Monks::Data and output its version like this:

use Monks::Data;

print $Monks::Data::VERSION;
[download]

There's no real harm in adding a version() method to your module but neither is there any real benefit unless you are inheriting from a superclass and want an easy way for multiple levels of inherited classes all to report the same version, or at least refer to the parent/grandparent version somehow. I'm struggling to think of a use case like this; there probably is one somewhere but it would be a fairly rare occurrence.

Firstly, a few words about some OO relationships. From your usage of terms like "parent", "child" and "subclass", I'm not convinced that you've fully grasped their meanings in the OO context.

Inheritance

This refers to what's called an IS-A relationship. For instance, in English, a Sports Car is a Car.

Equivalently, in Perl, the class SportsCar is a subclass of Car; SportsCar inherits from Car; Car is a parent class of SportsCar; SportsCar is a child class of Car; and @SportsCar::ISA should have Car as one of its elements.

Composition

This refers to what's called a HAS-A relationship. For instance, in English, a Car has an Engine; a Car is composed of a number of components one of which is an Engine; a Car is incomplete, and will not function as intended, without an Engine; however, an Engine is not a Car.

Equivalently, in Perl, the class Car needs to load the class Engine to function normally; however, the class Engine is NOT a subclass of Car; Engine DOES NOT inherit from Car; Car is NOT a parent class of Engine; Engine is NOT a child class of Car; and @Engine::ISA should NOT have Car as one of its elements.

Aggregation

This is also a HAS-A relationship but, unlike Composition, it is optional rather than essential. For instance, in English, a Car might have a Radio; a Car can have optional components one of which could be a Radio; a Car is complete, and will function as intended, without a Radio; as with an Engine, a Radio is not a Car.

Equivalently, in Perl, the class Car might conditionally load the class Radio; although, it is not required to function normally; the class Radio is NOT a subclass of Car; Radio DOES NOT inherit from Car; Car is NOT a parent class of Radio; Radio is NOT a child class of Car; and @Radio::ISA should NOT have Car as one of its elements.

Now let's change your original class names to more meaningful ones; perhaps Taxidermist and Taxidermy::Data. It should be fairly obvious that Taxidermy::Data is not a Taxidermist: there's no IS-A relationship here. However, a Taxidermist will have Taxidermy::Data; this is a HAS-A relationship and, as such, there are no parent, child or subclasses involved.

Going back to your original names, and refactoring the code to completely decouple Monks from Monks::Data, here's how you might implement a HAS-A relationship. (Again, the code is simplistic just to show a technique; production code should include validation, error checking and so on.)

In the script (pm_1169455_oo_class_example.pl), I read the name of the data module directly from the command line. This could also be done via options, config files, etc. I've only changed one line:

my $obj = Monks::->new;
[download]

to

my $obj = Monks::->new(data => $ARGV[0]);
[download]

The module Monks::Data is completely unchanged.

I've created another data module Monks::DataWeird, to test the decoupling, which looks like this:

package Monks::DataWeird;

use strict;
use warnings;

our $VERSION = '20160808.00';

sub get_furries { [qw{Rabbits Alligators Minks}] }

sub get_texture_for { +{qw{Rabbits soft Minks supersoft Cats scales}} 
+}

1;
[download]

The Monks module no longer contains any reference to Monks::Data or, indeed, any Monks::* module. It now looks like this:

package Monks;

use strict;
use warnings;

our $VERSION = '0.01';

sub new {
    my ($class, @args) = @_;
    my $self = bless { @args } => $class;
    $self->_init;
    return $self;
}

sub _init {
    my ($self) = @_;
    eval "require $_[0]->{data}";
}

sub _get_data { $_[0]->{data} }

sub is_furry {
    my ($self, $furry) = @_;
    grep { /$furry/ } @{$self->_get_data->get_furries};
}

sub find_fur_texture {
    my ($self, $fur) = @_;
    $self->_get_data->get_texture_for->{$fur};
}

1;
[download]

Here's three sample runs: two with the real data modules and one with a bogus, non-existent module.

$ pm_1169455_oo_class_example.pl Monks::Data
Cats: furry? Yes
Alligators: furry? No
Rabbits: furry? Yes
Fur texture for Cats: coarse
[download]

$ pm_1169455_oo_class_example.pl Monks::DataWeird
Cats: furry? No
Alligators: furry? Yes
Rabbits: furry? Yes
Fur texture for Cats: scales
[download]

$ pm_1169455_oo_class_example.pl XMonks::Data
Can't locate object method "get_furries" via package "XMonks::Data" at
+ Monks.pm line 24.
[download]

Just to briefly cover your other points:

"I'm sort of partial to BEGIN blocks, ..."

I have no problem with using BEGIN (or similar) blocks. I didn't use one as I saw no need. See ++hippo's response. Also look at the doco for the our function: it's usage and scoping rules seem to trip up a few users.

"Your guess is correct regarding $self->get_attr vs. $self->{attr}. I personally find the latter a lot easier to use, ..."

See the code for the Monks module. The "latter" form is only used for initialisation (_init()) and the accessor (_get_data()); elsewhere the accessor is used. Do not use the direct access method in other modules or scripts. Although it's usual to bless a hashref for Perl modules, that's not universal by any means: for example, IO::Handle uses a globref and Class::Std uses a scalarref. Even when the object remains a blessed hashref, the accessor could still change: as a contrived example, instead of $self->{attr}, it could become something like $self->{attr}[USER] // $self->{attr}[DEFAULT].

"Sorry about the furry/fluffy thing, I was just trying to have some fun with my sample code. :) "

No need to apologise at all. Changing everything to furry was just expedient for me.

I think my initial comments on "OO relationships", and subsequent refactoring, probably cover your other points.

[Aside: I'm not requesting you make changes, but for future reference take a look at "What shortcuts can I use for linking to other information?" and "Writeup Formatting Tips". Using links (e.g. parent, instead of 'parent') and marking up inline code (e.g. $self->{attr}, instead of $self->{attr}) can improve readability.]

— Ken

A second general guideline is that if you want to make the data interchangeable, don't use direct access to fields, encapsulate the data in method calls. To that end, either move the methods sub check_fluffy and sub find_fur_texture into package Monks::Data;, or wrap the fields into accessor methods like sub get_fluffy and sub get_textures, then the methods Monks::check_fluffy and Monks::find_fur_texture can call those methods to get the data.

Next, two suggestions for loosening the coupling between package Monks; and package Monks::Data;. Option A, don't use subclassing, instead make the data object Monks::Data a field on the class Monks - again encapsulated in a getter/setter method. Then that object could be passed to the constructor: my $object = Monks->new( data => Monks::Data->new );. That way, Monks doesn't need to care about the details of the data class, it simply needs to know that the data class has two methods e.g. get_fluffy and get_textures.

Option B, if you really want to use super/subclasses (whether that's the best design choice in this case is debatable, depending on what the rest of your class structure looks like), have package Monks; define abstract methods within itself that must be overridden in the subclasses. So for example, in your package Monks; add sub is_fluffy { croak "abstract method must be overridden" }, and in package Monks::Data; add sub is_fluffy { my $self = shift; my $who = shift; return 1 if grep { /\Q$who/ } qw/Rabbits Minks Cats/; return; }. Then, the calling script needs to do my $object = Monks::Data->new; instead of Monks->new. Now, the interesting thing about this approach is that other methods in Monks can still call the abstract methods in Monks - for example, in Monks, you can do sub check_fluffy { my $self = shift; return $self->is_fluffy(shift) ? "Yes" : "No"; }.

The above concepts apply to any OO design, so if that's what you're practicing, then using Moose or Moo just saves you some typing. Also, BTW, objects don't usually use Exporter.

Anon, thanks for the response. The main problem I'm trying to solve is that I have two classes of data (pardon the pun). I have some data and methods that are fairly static, those are going into the main package file. Then I have some data that is more variable, which I want to have in a separate file (and I mean file in the literal sense here) so that it can be more easily be regenerated. In my (admittedly naive) view, it seems that a subclass would be a good way to handle that, but I am definitely open to other suggestions.

In regards to not allowing direct access to fields, that's what I'm working on, or trying to at least. :) What I'm trying to figure out is what's the best way to go about that. I like the idea that Athanasius floated about "hiding" the static data in private variables, and creating methods in the subclass that do the work I'm currently doing in the main package that needs that data. If I understand what you're saying correctly, that seems to be what you're proposing as well?

Honestly, I don't understand your third paragraph, but hopefully what I've said above clarifies why I think I need a subclass.

The solution of the fourth paragraph muddies the water between the static and the dynamic data, that I described above. And thanks for the tip on Exporter, apparently I was too liberal with my copy and paste.