Re: Conditional and opt-in breaking change: is this design viable and my use of 'import' OK?

[choroba]

For object oriented modules, your approach is fragile and non standard.

Usually, when you need to instantiate objects, but the objects could be of several different classes based on what the user needs, you can use the Builder pattern. When building the builder object, you pass it all the arguments common to all the constructors of the different classes it can instantiate, it can then have several builder methods, one for each class, which take the arguments specific for the class's constructor.

If some of the methods have the same implementation in multiple classes, create an abstract parent class from which all the classes inherit (Foo::Common below), or use roles to implement the methods (not shown in the example below).

OO classes don't usually export anything. To import constants, use a dedicated class.

For a small project, this might seem like a Java-level verbosity. For larger projects, one would get mad at handling the different flags and ternaries in method implementations; with the Builder pattern you always know where to look when you need to see the implementation of a particular behaviour - and if it's not in the file, it tells you where to look further (parent or role).

I've used this at a $job - 2 with tens of constructor parameters, built on Moose. It worked great.

# ./lib/Foo/Classic.pm
# --------------------
package Foo::Classic;
use warnings;
use strict;
use feature qw{ say };
use experimental qw( signatures );

use parent 'Foo::Common';

sub style($self) { 'classic' }
sub m2($self)    { say 'classic 2' }

__PACKAGE__

# ./lib/Foo/Constants.pm
# ----------------------
package Foo::Constants;
use warnings;
use strict;

use Exporter qw{ import };

my %const;
BEGIN {
    %const = (
        ABC => 'abc',
        XYZ => 'xyz',
    );
}
use constant \%const;
our @EXPORT_OK = keys %const;

__PACKAGE__

# ./lib/Foo/Modern.pm
# -------------------
package Foo::Modern;
use warnings;
use strict;
use feature qw{ say };
use experimental qw( signatures );

use parent 'Foo::Common';

sub style($self) { 'modern' }
sub m2($self)    { say 'modern 2' }

__PACKAGE__

# ./lib/Foo/Builder.pm
# --------------------
package Foo::Builder;
use warnings;
use strict;
use experimental qw( signatures );

use Foo::Classic;
use Foo::Modern;

my %CLASS = (classic => 'Foo::Classic',
             modern  => 'Foo::Modern');
sub new($class) {
    return bless {}, $class
}

sub build($self, $style, @args) {
    my $class = $CLASS{$style};
    die "Unknown style $style" unless $class;
    return $class->new(@args)
}
__PACKAGE__

# ./lib/Foo/Common.pm
# -------------------
package Foo::Common;
use warnings;
use strict;
use feature qw{ say };
use experimental qw( signatures );

sub new($class)  { bless {}, $class }
sub m1($self)    { say $self->style, ' 1' }
sub style($self) { die 'Not implemented' }

__PACKAGE__

# ./script.pl
# -----------
#!/usr/bin/perl
use warnings;
use strict;
use feature qw{ say };

use lib 'lib';
use Foo::Builder;
use Foo::Constants qw( ABC XYZ );

my $builder = 'Foo::Builder'->new;

my $m = $builder->build('modern');
$m->m1;
$m->m2;

my $c = $builder->build('classic');
$c->m1;
$c->m2;

say ABC;

Also note that this approach addresses my other comment: you can easily instantiate both the classic and modern objects in the same program.

[Anonymous Monk]

Thank you very much for detailed answer, I appreciate. Never seen __PACKAGE__ for "true" at the end of module, learning something new each day.

Constants go to dedicated module and "modern" user is told to "use" it. Gratefully accepted.

To answer your other comment: yes, the OOP, shared code/behaviour and separate data/properties, I have some vague idea. If "import" redefines the subroutines, it's once and for all. Though I didn't expect such mixed usage scenario, I planned to eventually do at least something about it, like (assuming $MODE undefined initially):

sub import {
    my ( $self, $flag ) = @_;
    
    if ( defined $flag and $flag eq ':modern' ) {
        if ( defined $MODE ) {
            if ( $MODE == 0 ) {
                warn "Note: Foo is in classic mode\n"
            }
        }
        else {
            $MODE = 1;
            Foo-> export_to_level( 1, 'Foo', keys %const );
            
            no warnings 'redefine'; 
            *Foo::m2 = \&Foo::Modern::m2;
            # etc.
            # longer list
        }
    }
    else {
        if ( defined $MODE ) {
            if ( $MODE == 1 ) {
                warn "Note: Foo is in modern mode\n"
            }
        }
        else {
            $MODE = 0
        }
    }
}

If someone does just (or mixes with other uses) "require Foo;" or "use Foo();" it'll be considered an act of direct sabotage ;-) But in the end I'll probably abandon the idea to abuse "import", therefore doesn't matter. I don't understand, however, what's the benefit of "Builder". The "script.pl" is written by the end user, all she wants is simply "use Foo;". If asked instead from this day on to:

use Foo::Builder;
my $builder = 'Foo::Builder'->new;
my $object = $builder->build('classic');

then I'm afraid to be pointed at the door. Or if in less grumpy mood/mode, she'll agree to hit a few more keys to "use Foo::Classic;". And if "modern" user happens nearby, he'll say "Aha, I'll do use Foo::Modern; use Foo::Constants; then!"

I was hoping to use a single ternary to check a flag in, say, 60 lines method (and there are several such methods) so that 2 of these lines are executed in this or that mode; instead of keeping almost exact duplicates in two packages. I'll think more about it and ask further. Thank you.