Role-Basic icon indicating copy to clipboard operation
Role-Basic copied to clipboard

DOES: return all roles in array context

Open mlawren opened this issue 1 year ago • 6 comments

I wanted to run some initialization code for each role consumed when instantiating an object. The composition data is private to Role::Basic so I can't inherit from it to override. This pull request exposes the composition by extending DOES in a way it was never tested for, presumably maintaining backwards compatibility.

mlawren avatar Jan 08 '24 19:01 mlawren

Already found my first challenge. I didn't know that the Universal package has a DOES method. That one requires an argument, and is possibly what you modelled Role::Basic's DOES on. Providing a different interface is not a good idea.

Would you accept a patch that adds a ROLES method?

mlawren avatar Jan 08 '24 20:01 mlawren

As for adding a ROLES method, it seems like this should be a method that the consumer should be able to exclude in the unlikely event they have their own ROLES method. That means it should also be added to requirements inside the code.

Ovid avatar Jan 09 '24 08:01 Ovid

As for adding a ROLES method, it seems like this should be a method that the consumer should be able to exclude in the unlikely event they have their own ROLES method. That means it should also be added to requirements inside the code.

I don't understand what you mean exactly, and the various scenarios feels somehat complicated. Are you simply proposing something like this for classes?:

use Role::Basic 'with', 'NO_ROLES'

And/or this for roles?:

use Role::Basic 'NO_ROLES'

If a role/class wants to provide it's own ROLES method, is the assumption that it has nothing to do with Role::Basic composition? Or should it be considered the authoritative composition source, resulting in something like this:

*{$target .'::ROLES'} = sub {
    $target, map {
        if ($_->can('ROLES')) {
            $_->ROLES
        } else {
            $_
        }
    } keys %{$HAS_ROLES{$target}}
}

mlawren avatar Jan 09 '24 10:01 mlawren

I feel I should try a bit harder to avoid the ROLES() can of worms. How about this as an interface?

my @roles = My::Class->DOES('*');

Array context only, using a shell-like glob, which can't match a package name.

mlawren avatar Jan 09 '24 11:01 mlawren

Internally, we have a %REQUIRED_BY hash which lists the methods the role requires. Any method that is excluded is still automatically required because that's part of the contract (and the role might call the method internally).

If we provide our own ROLES method, even if the role itself doesn't implement it, code using Role::Basic must assume the method is there. Thus, we need to check for method conflicts. The class might want to exclude the ROLES method because it already has one (admittedly, this is unlikely). If the ROLES method is excluded, it's still part of the contract of the role and thus, it's a required method.

See line 66 and line 263 for (some) examples.

Ovid avatar Jan 09 '24 13:01 Ovid

You're MLAWREN on CPAN, yes? If so, I'd be happy to give you comaint of this module.

Ovid avatar Jan 09 '24 13:01 Ovid