Beefy Boxes and Bandwidth Generously Provided by pair Networks
more useful options
 
PerlMonks  

Conditional inheritance strategy

by citromatik (Curate)
on Oct 27, 2010 at 11:50 UTC ( #867690=perlquestion: print w/ replies, xml ) Need Help??
citromatik has asked for the wisdom of the Perl Monks concerning the following question:

Hi all

I have a class that can create an object using data coming from different sources / formats. Like:

package My sub new { my ($class,%args) = @_; my $self = bless {}, $class; if ($args{source} eq 'xml') { require My::XML; @ISA = qw/My::XML/; } elsif ($args{source} eq 'json') { require My::JSON; @ISA = qw/My::JSON/; } $self->_init(); # Versions in both My::XML and My::JSON return ($self); } # rest of methods sub method1 { ... } sub method2 { ... }

The packages My::XML and My::JSON:

package My::XML; sub _init { ... } 1; package My::JSON; sub _init { ... } 1;

The calling code would be:

use My; my $obj = My->new(source=>"xml"); # Do whatever with $obj

Is this a correct design to face this? Any caveat you may anticipate? Are there better alternatives?

Thank you very much in advance for your help

citromatik

Comment on Conditional inheritance strategy
Select or Download Code
Re: Conditional inheritance strategy
by bart (Canon) on Oct 27, 2010 at 12:20 UTC
    That looks fine to me. There are similar mechanisms in use on CPAN, such as: You could check how they did things, as a source of inspiration.

    One caveat: you don't have any warning if the "source" isn't recognized.

Re: Conditional inheritance strategy
by Corion (Pope) on Oct 27, 2010 at 12:26 UTC

    Why have package My:: inherit from either My::XML or My::JSON at all? I would have My::new() return either a My::XML or My::JSON. Maybe both, My::XML and My::JSON can inherit from My::Base if they really share common routines.

      Thanks a lot for your thoughts

      In fact I followed your last suggestion in a first version of the classes (My::XML and My::JSON inherited from My::Base). But I found that doing it in that way you emphasize how the data is stored (XML or JSON in this case). But in both cases you should be able to end up with the same object regardless how the information to build it was stored. I mean, it should be natural to get an object of class My instead of My::XML or My::JSON.

      I don't know if this has any sense.

      citromatik

        Maybe you want then My->from_xml and/or My->from_json? Why is it important that My objects are XML or JSON objects? It seems to me that they should maybe be able to display or store themselves as XML or JSON, which I would put into a separate plugin or serialization class.

        What you are doing is basically a factory design pattern where the factory (in your case the My:: package) usually is not part of the hierarchie of the classes it creates.

        I mean, it should be natural to get an object of class My instead of My::XML or My::JSON.
        I am not sure if I understand your question, but clients of your factory would usually not bother weather they have a XML or a JSON instance but work through the interface that the base-class (My::Base) provides.
Re: Conditional inheritance strategy
by kcott (Abbot) on Oct 27, 2010 at 12:54 UTC

    Basically this looks good. Here's a few thoughts and suggestions:

    • Your if-elsif has no else. In this case, handling missing or unknown source (as alluded to by bart) would be appropriate.
    • Consider making the source types case-insensitive.
    • Do you need to pass %args to _init()?
    • You may find it useful to have an _init() in My which calls $self->SUPER::_init().
    • Is there a reason to return $self in list context?

    -- Ken

      Thanks for your answer.

      As for your specific suggestions, the code I posted is a simplification of the actual code that I have (where most of your thoughts have been addressed). I was interested in possible issues using the strategy.

      Thanks again

      citromatik

Re: Conditional inheritance strategy
by BrowserUk (Pope) on Oct 27, 2010 at 13:02 UTC

    Hm. It's not possible to tell from the posted snippets whether this really makes sense.

    My instinct tells me that the XML or JSON is only used to initialise the objects of class My, and that once so initialised, the instances from either source are essentially identical. If so, then I would deem what you are doing as "unnecessary inheritance".

    Based upon the above assumption, I would suggest that by using inheritance here, you are loading--and persisting--the overheads of the XML or JSON classes into each instance of My class for no good reason. That is, you are forcing each instance of My class to carry around the baggage--instance data and class references--of whichever base class was used to initialise it. And for no good reason, as that baggage will never be used again during the life of the My class instance.

    If so, if you instantiated an instance of the relevant XML or JSON class to just parse the source data; then retrieved the data required by My class; and then allowed the parse instance to be reclaimed as soon as the initialisation of a new My class instance was complete; then your My class instances might be individually smaller. And possibly more efficien,t because of shorter method resolution paths.

    Hard to tell from what you've posted, but worth considering.


    Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
    "Science is about questioning the status quo. Questioning authority".
    In the absence of evidence, opinion is indistinguishable from prejudice.
      Based upon the above assumption, I would suggest that by using inheritance here, you are loading--and persisting--the overheads of the XML or JSON classes into each instance of My class for no good reason. That is, you are forcing each instance of My class to carry around the baggage--instance data and class references--of whichever base class was used to initialise it. And for no good reason, as that baggage will never be used again during the life of the My class instance.
      I wonder what baggage you are referring to. I would presume to point of _init is to set up instance data - why else call a method named _init from new? Of course, if said data is never used, it's pointless, but the code fragment doesn't suggest so. As for a class reference, any object will have a class reference - a single string (package name). The reference doesn't increase depending whether inheritance is used or not. But even if I misunderstood you, your arguments seem to be against using inheritance in general, and not specific to the OP.

        If nothing else, having the initialisation class in the inheritance tree via @ISA, is baggage, if there are no other methods than _init(). Looking back at the OPs code, there seems to be little reason for calling the _init() routines as methods rather than subroutines.

        Indeed, as no instance of the initialisation classes is ever instantiated, it could equally be termed "poor OO". And it is a weird use of "inheritance" in every sense. If the idea is simple to provide two methods of initialisation, better I would say to have them both in the main My package space called (say) _initFromXML() and _initFromJSON() and ditch the extra package spaces entirely.

        But, as I said, it is quite hard to tell from what the OP posted, as he has obviously simplified the code for the purpose of the question. It is always possible that he also has code in each base package to persist the objects to disk or database, which might make all the difference.


        Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
        "Science is about questioning the status quo. Questioning authority".
        In the absence of evidence, opinion is indistinguishable from prejudice.

      Hi BrowserUk,

      You are right in your assumptions. My::XML and My::JSON only provide data to the inherited class, no other methods are used (except methods internally used by My::XML or My::JSON).

      If so, if you instantiated an instance of the relevant XML or JSON class to just parse the source data; then retrieved the data required by My class; and then allowed the parse instance to be reclaimed as soon as the initialisation of a new My class instance was complete; then your My class instances might be individually smaller. And possibly more efficien,t because of shorter method resolution paths.

      Sorry if I misunderstand your. Are you suggesting the following?:

      Package My:

      sub new { my ($class,%args) = @_; my $data; if ($args{source} eq 'xml') { require My::XML; # @ISA = qw/My::XML/; $data = My::XML->new(%args); } elsif ($args{source} eq 'json') { require My::JSON; # @ISA = qw/My::JSON/; $data = My::JSON->new(%args); } my $self = bless \%{$data}, $class; return ($self); } # rest of methods sub method1 { ... } sub method2 { ... } 1;

      Package My::XML

      package My::XML; use XML::Simple qw(:strict); sub new { my ($class,%args) = @_; my $self = bless {}, $class; $self->_init(); return $self; } sub _init { # Code to read the data from XML source } 1;

      Same for My::JSON

      Thank you very much for your comments

      citromatik

        Are you suggesting the following?: [ ... CODE ... ]

        Yes, that is pretty much what I was suggesting--but only because I assumed that the XML & JSON modules contained classes. Ie. That there was a purpose in inheriting from them because they had local state and more methods than just _init().

        With your confirmation that they only contain one _init() subroutine each--although you are calling them as methods--personally, I can see no reason not to simply move the two _init() routines into your My package (with different names), and drop the extra packages.


        Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
        "Science is about questioning the status quo. Questioning authority".
        In the absence of evidence, opinion is indistinguishable from prejudice.
Re: Conditional inheritance strategy
by JavaFan (Canon) on Oct 27, 2010 at 13:02 UTC
    Is this a correct design to face this?
    Not in general. @ISA is a package variable, it isn't bound to a specific instance. So, if you do:
    my $x = My->new(source=>"xml"); my $y = My->new(source=>"json");
    then now both $x and $y are instances of a class that inherits from My::JSON. If My::XML and My::JSON only contain _init, and this is only called from new, you're getting away with it, but if they also contain methods that will be called elsewhere from the program, behaviour may be unexpected.

    Perhaps you can reverse the inheritance order:

    package My::XML; use My; our @ISA = qw[My]; sub init {...} 1; package My::JSON; use My; our @ISA = qw[My]; sub init {...} 1; package My; sub new { my $class = shift; my $self = bless {}, $class; $self->init; $self; } 1; package main; use My::XML; use My::JSON; my $x = My::XML->new; my $y = My::JSON->new;
      Not in general. @ISA is a package variable, it isn't bound to a specific instance. So, if you do:
      my $x = My->new(source=>"xml"); my $y = My->new(source=>"json");
      then now both $x and $y are instances of a class that inherits from My::JSON.

      Good point

      If My::XML and My::JSON only contain _init, and this is only called from new, you're getting away with it, but if they also contain methods that will be called elsewhere from the program, behaviour may be unexpected.

      Yes, My::XML and My::JSON only contain _init and this is only called from new, so this is not a problem, but I think that I will reverse the inheritance as suggested several times in this thread

      Thanks

      citromatik

Re: Conditional inheritance strategy
by jethro (Monsignor) on Oct 27, 2010 at 13:05 UTC

    If I get you correctly, you want each object either to be inherited from My::XML or My::JSON, but that doesn't work, as inheritance is a configuration at the class level

    One solution would be to store a reference to an object of type My::XML or My::JSON into your objects and call methods from there.

    Or you could simply have two classes of type My::XML and My::JSON that both inherit from My

Re: Conditional inheritance strategy
by roboticus (Canon) on Oct 27, 2010 at 13:27 UTC

    citromatik:

    It sounds as if the subclasses are simply artifacts of the format of the data source, and wouldn't have any behavioral differences otherwise. If that's the case, I wouldn't make the objects different types. Instead, I'd provide reader classes that read My objects, like My::Reader::XML.

    ...roboticus

    Update: Consider this a "me, too" node, as I missed BrowserUk's node above.

Re: Conditional inheritance strategy
by ikegami (Pope) on Oct 27, 2010 at 16:36 UTC

    Don't have My inherit from My::XML and My::JSON, have My::XML and My::JSON inherit from My.

    I'm guessing you want to do this since you get a file name and you want to load the file using the right module.

    package My; sub load { my ($class, %args) = @_; my $file_type = ...; if ($file_type eq 'xml') { require My::XML; return My::XML->new(%args); } elsif ($file_type eq 'json') { require My::JSON; return My::JSON->new(%args); } } sub new { ... } sub method1 { ... } sub method2 { ... }
Re: Conditional inheritance strategy
by pajout (Curate) on Oct 27, 2010 at 18:02 UTC
    Answer 1: Why not, if it does work for you...

    Answer 2: Perhaps I am too rigid, but I think that ascendant would not know anything about it's descendants.

Re: Conditional inheritance strategy
by mojotoad (Monsignor) on Oct 28, 2010 at 07:57 UTC
    You have a view: standard methods for your object flavor

    You have a control: what you do with said well-behaved objects

    You have a model: how stuff is retrieved and/or stored -- in this case XML or JSON or whatever, summon the demon from the depths and make it dance. DANCE I say.

    Yes, it's true that purists will object to my interpretation, but they drank the cool-aid. I drank the cup.

    Cheers,
    Matt

Re: Conditional inheritance strategy
by happy.barney (Pilgrim) on Oct 28, 2010 at 09:52 UTC
    few notes:
    use dispatch map
    our $dispatch_map = { xml => 'My::XML', json => 'My::JSON', }; sub new { ... die unless exists $dispatch_map->{$args{source}}; my $class = $dispatch_map->{$args{source}} Module::Load::load ($class); $self->{engine} = $class; # or $class->new ($self); }
    you can call foreign methods ...
    $self->My::JSON::_init; # or my $method = $self->{engine} . '::_init'; $self->$method;

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://867690]
Approved by Corion
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others taking refuge in the Monastery: (2)
As of 2014-09-20 00:27 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    How do you remember the number of days in each month?











    Results (151 votes), past polls