[bop-devel] (eval/AUTOLOAD and the problem as I see it)

[Phil Lobbes]

Follow up comments from my original email and Ivan's response...

[ ... other text related to patch deleted ... ]
Phil> * get rid of possible use an undefined value as a HASH reference
Phil> * stop redefining subs if they already exist
Phil>   - NOTE: it might be nice to do away with this completely and use a
Phil>     notion of $self->param('name' => 'value') and
Phil>     $self->delete('name') instead of creating subs on the fly.  This
Phil>     would be a little cleaner design for v3 I would think.  It would
Phil>     get rid of some (in theory) unnecesary subs and remove some uses
Phil>     of eval which might be a little better for performance (probably
Phil>     minor but haven't benchmarked to know for sure) and perhaps a
Phil>     bit more friendly to people that use mod_perl too.

Ivan> I don't agree we should do away with this completely - I think we
Ivan> should think *really* long and hard about API changes that affect
Ivan> B:OP users / app writers in a backwards-incompatible way.

I agree, but I also don't want to simply concede that we should keep
something because "that's the way it has [always] been".

Ivan> If you felt it was important, perhaps we could get rid of the
Ivan> string eval's without breaking the API or processor modules by
Ivan> using an AUTOLOAD sub?

While I don't love the eval, nor even using AUTOLOAD for this usage.
The bigger issue I see is that we're modifying the class itself and for
people like me when I switch between drivers in mod_perl I don't want
pseudo accessors from another driver getting created for my object and I
doubt any other drivers want mine for that matter.  NOTE: At the bottom
of this message you will find some test code to help clarify.

Because of this type of issue I think we're better off changing our way
of dealing with this in a different way and the param()/delete()
paradigm used with CGI, CGI::Application and other modules seems like a
pretty good fit.

Ivan> Why is this unfriendly to mod_perl users?  I use B:OP with
Ivan> mod_perl all the time, and this was never a problem for me.

My usage may be a little unique, but hopefully you can relate to why I
dislike this a little more now.

Ivan> My most most annoying peeve with mod_perl usage is when
Ivan> Crypt::SSLeay or Net::SSLeay loads up a different OpenSSL library
Ivan> verison that the one compiled into the web server and the whole
Ivan> thing segfaults without a trace of what happened.  :)

That really is annoying, certainly a segfault is worse than a little
namespace pollution :-(.

Phil

Details on the test cases that show namespace pollution...

$ make test TEST_VERBOSE=1
bop_class.t
PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e"
"test_harness(1, 'blib/lib', 'blib/arch')" t/*.t
t/bop_class....1..9
ok 1 - use Business::OnlinePayment;
ok 2 - MOCK1 CLASS can proc1
ok 3 - MOCK1 OBJECT can proc1
ok 4 - MOCK1 CLASS can NOT proc2
ok 5 - MOCK1 OBJECT can NOT proc2
ok 6 - MOCK2 CLASS can proc1 (BAD!)
ok 7 - MOCK2 OBJECT can proc1 (BAD!)
ok 8 - MOCK2 CLASS can proc2
ok 9 - MOCK2 OBJECT can proc2
ok

Code for bop_class.t:
-----------------------------------------------------------------

#!/usr/bin/perl

use strict;
use warnings;
use Test::More tests => 9;

BEGIN { use_ok("Business::OnlinePayment") or exit; }

{    # fake test driver #1

    package Business::OnlinePayment::MOCK1;
    use strict;
    use warnings;
    use base qw(Business::OnlinePayment);
}

{    # fake test driver #2

    package Business::OnlinePayment::MOCK2;
    use strict;
    use warnings;
    use base qw(Business::OnlinePayment);
}

# trick to make use() happy (called in Business::OnlinePayment->new)
$INC{"Business/OnlinePayment/MOCK1.pm"} = "testing";
$INC{"Business/OnlinePayment/MOCK2.pm"} = "testing";

{    # MOCK1: new (via build_subs) automatically creates accessors for args
    my $obj = Business::OnlinePayment->new( "MOCK1", "proc1" => "value1" );

    is( ref Business::OnlinePayment::MOCK1->can("proc1"),
        "CODE", "MOCK1 CLASS can proc1" );
    is( ref $obj->can("proc1"),
        "CODE", "MOCK1 OBJECT can proc1" );

    is( Business::OnlinePayment::MOCK1->can("proc2"),
        undef, "MOCK1 CLASS can NOT proc2" );
    is( $obj->can("proc2"),
        undef, "MOCK1 OBJECT can NOT proc2" );
}

{    # MOCK2: new (via build_subs) automatically creates accessors for args
    my $obj = Business::OnlinePayment->new( "MOCK2", "proc2" => "value2" );

     # prove MOCK2 namespace is polluted...
    is( ref Business::OnlinePayment::MOCK2->can("proc1"),
        "CODE", "MOCK2 CLASS can proc1 (BAD!)" );
    is( ref $obj->can("proc1"),
       "CODE", "MOCK2 OBJECT can proc1 (BAD!)" );

    is( ref Business::OnlinePayment::MOCK2->can("proc2"),
        "CODE", "MOCK2 CLASS can proc2" );
    is( ref $obj->can("proc2"),
        "CODE", "MOCK2 OBJECT can proc2" );
}