Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add methods to allow modification of class and role module names #68

Closed
wants to merge 2 commits into from

Conversation

djerius
Copy link
Contributor

@djerius djerius commented Feb 1, 2019

I'd like to make it easier for the user to specify class and roles names. Mojolicious, for example, allows one to specify roles using a + prefix to indicate the name is relative to some other package.

This commit add two method to Beam::Wire: resolve_class and resolve_role which are passed the specified name and return the fully resolved name. (I created separate versions as there might be different rules for them.)

@coveralls
Copy link

Coverage Status

Coverage increased (+1.1%) to 93.993% when pulling 330806e on djerius:resolvemodule into 41027e2 on preaction:master.

@preaction
Copy link
Owner

preaction commented Feb 7, 2019

This sounds like a good feature to add, but I'm not sure I like the idea of adding methods that don't do anything and are meant to be subclassed. I think there's a better way: Fire a configure_service event and allow the event to edit the service's configuration. That way you could have an event handler that edits the service's configuration before it's created and resolve the class/role or indeed do whatever you want.

I'm imagining something like:

use v5.28;
use warnings;
use experimental qw( signatures );
use Beam::Wire;

my $wire = Beam::Wire->new(
    config => {
        service => {
            '$class' => '+Service',
        },
    },
);

$wire->on(
    configure_service => sub ( $wire, $service_name, $config ) {
        if ( $config->{class} ) {
            $config->{class} =~ s/^\+/Local::Project::/;
        }
    },
);

You could still create a subclass if you wanted, and that subclass could use the event:

package My::Wire;
use v5.28;
use Moo;
use experimental qw( signatures );
extends 'Beam::Wire';
sub BUILD ( $self ) {
    $self->on( configure_service => \&_resolve_class );
}
sub _resolve_class ( $wire, $service_name, $config ) {
    ...;
}
1;

Does this sound like it will achieve what you want?

@djerius
Copy link
Contributor Author

djerius commented Feb 7, 2019

I guess it's a question of style. 99% of the work I do is non-event driven, so it's more intuitive to me to use roles with method modifiers or inheritance to change behaviors. I don't normally associate DI frameworks with events, so it seems a bit foreign to use that approach to modify its behavior.

@preaction
Copy link
Owner

In my opinion they're functionally the same, but event hooks provide for a (subjectively) cleaner null case and don't require a subclass to add the desired functionality. In a language that lacks anonymous functions and/or first-class subroutines, there's only the subclassing option, but we've got Perl 😄.

Adding an event would also be effectively the same as subclassing and overriding the get_config method, which should work for you as well. If it doesn't, I'm sure some adjustment could be made (and a test written to ensure future compatibility, OO design accounting for subclassing can be hard...)

preaction added a commit that referenced this pull request Mar 23, 2019
These events allow some management of services, and for extending the
configuration syntax.

Refs #68
@preaction
Copy link
Owner

I've added the events to the module in version 1.023. Did you want to give it a try to see if it works for your case?

@preaction preaction closed this Mar 23, 2019
@djerius
Copy link
Contributor Author

djerius commented Mar 25, 2019

I'll give it a whirl shortly. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants