One feature of Moose roles that is if a role provides a method and a class uses that role, if a class defines a method that the role provides, it silently overrides the role's version of said method. This makes it easy to customize your behavior per class.
However, did you get the "silently overrides" part? Regardless of this being in the original traits papers, how it's implemented in Perl 6 and how it's implemented in Moose roles, it's a mistake. I have again lost quite a bit of time debugging this "feature". I'm really tired of this silent overriding of behavior. This is what roles are supposed to help with!
The Moose maintainers have not expressed a desire to change this (it's been discussed in this blog) and in fairness to them, Moose is absolutely fantastic in most regards. However, when working with the larger systems that roles help out with, it's ridiculously frustrating to keep getting bitten by this over and over. I've discovered that this is a common problem when converting a base class into a role. You would think that since the child class is overriding the base class, this would be a non-issue, but we often have calls to $self->next::method and everything falls down and goes boom.
It we didn't have a great test suite, this would have hurt our customers badly.
Not completely into Moose yet, but isn't a role the equivalent of a Java interface?
In that case, the behaviour you describe has been implemented there as a intended feature: a regular class *implements* the interface.
Additionally, a role seems to also have the function of an abstract class as well. In Java, overriding concrete methods is, again, a feature you want.
If you don't want to override a method simply do not include a overloading method in the implementing class.
It looks like a (sensible) design decision to me.
C.
Re:Is it a bug or a feature?
Ovid on 2009-04-14T16:09:59
Roles are far more than interfaces. I strongly recommend reading up on them to understand what they do. The protect (usually) against method collision and they provide a default implementation. If you want to use one like an interface, it's simple:
package MyRole::DoesSomething;
use Moose::Role;
requires qw(
save
search
);And now your classes (or some other role composed into said classes) must provide save and search methods.
However, if you have this:
package MyRole::DoesSomething;
use Moose::Role;
requires qw(
save
search
);
sub is_deletable {
my ($self,$id) = "_;
return if $self->search( children => $id );
return 1;
}And the class using the role also has an 'is_deletable' method, than this potentially very important behavior is silently discarded. You need to rewrite it as something like this:
package MyRole::DoesSomething;
use Moose::Role;
requires qw(
is_deletable
save
search
);
around is_deletable => sub {
my ($is_deletable,$self,$id) = "_;
return if not $self->$is_deletable;
return if $self->search( children => $id );
return 1;
};And that solves your problem. However, silently discarding behavior is not something I expect from roles. So yes, this is defined as a "feature", but it's one that has repeatedly led to many hours of debugging for me.
If there was simply a normal "method collision" when the roles are composed, I could easily resolve this in my role composition declarations and maintainers of my code would see it when they edit the file.
Re:Is it a bug or a feature?
nxadm on 2009-04-14T17:02:30
So it's indeed also like an Abstract class in Java. The solution for the problem in the Java world is the @Override annotation before the overriding method.
It overrides silently, but it's clear in your code and your IDE will warn you if you don't add the @Override annotation.
Something like that would indeed save debugging time.
C.
Re:Is it a bug or a feature?
chromatic on 2009-04-14T17:54:07
So it's indeed also like an Abstract class in Java.
A role is like an abstract class in Java in the same way lightning is like a lightning bug.
Re:Is it a bug or a feature?
Ovid on 2009-04-15T08:28:09
No, it's not like an abstract class. I can compose several roles into my class in Perl, but I can't multiply inherit from several abstract classes in Java. Further, if I have two roles which provide the same method name, this fails at composition time but roles (particularly with Moose) offer you a rich variety of ways of resolving conflicts. You don't get that benefit with abstract classes.
You also mention the silent overriding and the "clear in your code" and "your IDE will warn you". Two problems with this. First, if you add a method to an abstract class and miss one of the classes using it, it's not "clear in your code" because you never saw it. Plus, what if someone is using a different IDE than you are and the @Override annotation doesn't mean anything? Now you're not just forcing a programming language on me and all the programmers who follow you on the project (which isn't a bad thing), but you're also forcing an IDE on me and all the programmers who follow you on the project (which is a bad thing). Serious issues like this should be resolved in the language itself, not in a third-party tool.
Re:Is it a bug or a feature?
nxadm on 2009-04-15T22:04:07
I see your point about multiple inheritance.
However, annotations are a language feature and not something IDE-centric.
Re:Is it a bug or a feature?
Ovid on 2009-04-15T22:10:01
Ah, my apologies about misunderstanding the annotations. It's been too long since I've programmed Java (er, or something like that
:) Re:Is it a bug or a feature?
chromatic on 2009-04-14T17:58:33
And the class using the role also has an 'is_deletable' method, than this potentially very important behavior is silently discarded.
For a definition of "silently" which means "Code I explicitly wrote in my class itself", perhaps.
What if you extracted all behavior into roles and composed them into classes? I think that would produce the desired results.
Re:Is it a bug or a feature?
zby on 2009-04-14T18:13:25
Still if you redefine a sub in standard procedural code - just as explicitely - you'll get a warning if you have warnings on.Re:Is it a bug or a feature?
chromatic on 2009-04-14T20:10:55
Still if you redefine a sub in standard procedural code - just as [explicitly]...
If you override a method in OO code (the paradigm under discussion) explicitly, you get no warning or error.
Changing this default behavior is like resolving method dispatch at compile time. That's fine for certain languages, none of which have "Perl" in their names.
Re:Is it a bug or a feature?
zby on 2009-04-15T11:49:18
Overriding methods in subclasses is a standard technique - there is no warning, because it is needed and there is no other way. In the case of roles there is a different way (as the recently anounced solution shows). So why not have this useful warning?Re:Is it a bug or a feature?
chromatic on 2009-04-15T18:09:31
Class-local overriding of composed role methods is a standard technique too. It's been in the Perl 6 role specification for years. I have code which relies on it -- and that code predates Moose.
I fail to understand the value of adding a default warning for intended, specified, useful behavior -- especially behavior you can't accidentally back into.
Re:Is it a bug or a feature?
Ovid on 2009-04-15T19:18:48
We get warnings for undefined variables because that might be a problem. It's trivial to suppress if you really meant it.
If your class silently overrides a role's method, that might be a problem. By issuing a warning, I can be alerted to a potential issue and it's trivial to suppress the warning if I really meant it. I've lost too many hours debugging this silent behavior to think I'll be convinced otherwise.
Re:Is it a bug or a feature?
chromatic on 2009-04-15T22:11:02
If your class silently overrides a role's method....
How do you silently write a method? Every time I've written a method, I've done so deliberately.
Should there be a warning if you accidentally inherit from the wrong class? How about a warning if you mistype the name of your class? What if the name of the file and its directory path doesn't match the name of the package or class contained? I've made all of those mistakes, and they've all been difficult to debug -- but I don't know any usable heuristic which can distinguish between honest mistakes and deliberate design decisions.
Class-local implementations of provided role methods have always been in the specification. The design decision to allow them was very deliberate.
Re:Is it a bug or a feature?
mpeters on 2009-04-16T12:17:03
How do you silently write a method? Every time I've written a method, I've done so deliberately.
He didn't say silently write a method he said silently override a method. Big difference. On a big project if there are several classes using a Role and that Role in the future adds a method the author now has to check every class that uses that Role to make sure there are no name clashes. If he ads a method that clashes with other Roles being used at the same time he'll get an error. But if that clash is with an class method he gets no such help.
Now imagine the author of the Role has no control over classes that use it (they are in another project or the Role is on CPAN and is being used by other CPAN modules), now the author of the Role is stuck. He can't add any new methods to his Role. That's the potential problem that this warning will
.. um .. warn about. Re:Is it a bug or a feature?
Stevan on 2009-04-15T22:05:23
chromatic,FWIW, I agree with you, however a warning cannot hurt and can possibly make it easier to spot problems during heavy role refactoring. And if we find it is more annoying then expected and/or not as helpful as expected, we can remove it as easily as we put it in. The decision made on #moose was that it was a simple enough change and an innocuous enough thing that it was at least worth giving a try. Roles are still a new "thing" and so I still feel like we are exploring some unknown territory here even after using and exploring them for almost 5+ years now (I released Class::Trait 0.01 in 2004).
- Stevan
Re:Is it a bug or a feature?
chromatic on 2009-04-15T22:12:32
... a warning cannot hurt... That's completely untrue. This particular warning may not hurt (I don't believe that, but I acknowledge the possibility), but warnings can hurt. I should write a longer post about this. For now, consider take Vista's UAC for examples of supposedly helpful warnings that do, in fact, hurt.
Re:Is it a bug or a feature?
Ovid on 2009-04-15T02:26:56
You know, I've considered that, but only to work around this issue. I don't want to extract behavior into roles unless I intend and need for that behavior to be shared.
Re:Is it a bug or a feature?
Ovid on 2009-04-14T16:18:48
Oh, and I forgot to respond to this bit:
If you don't want to override a method simply do not include a overloading method in the implementing class.
Easier said than done. When I am overriding a role's method but I don't know I'm overriding it, I would like to know that. Really. It's important to me. That's a huge part of the OO inheritance mess that roles are designed to avoid and here's one relatively common edge case that is ignored. So maybe I should just remember all of my method names? I don't think so:
$ find lib/ -name '*.pm' |wc -l
549
If Moose developers won't change the default behaviour (I don't know the issues well enough to argue either way) then I'd hope they'd be open to providing a mechanism to let you do it yourself.
Perhaps you already can by sub-classing Moose::Role.
Re:Mechanism vs Policy
Ovid on 2009-04-14T16:11:27
I'm pretty sure I can subclass Moose::Role, but I've never tried. Sounds like the way to go.
And at this point, even if they did agree this behavior is wrong (I don't believe they do), this has been part of the interface long enough that I doubt they would change it (kind of like how Perl 5's SUPER:: bug has never been fixed).
Re:Mechanism vs Policy
perigrin on 2009-04-14T17:31:43
Note I am not pretending to even play a Moose core developer on TV. I'm just giving my two cents being an active part of the Moose developer community.
If it was decided that this was the right decision it would be changed with a long deprecation cycle. The Moose team isn't above deprecating bad features. However something like this would need to be "proven" in a MooseX:: class for a while I would think.
The right answer I think would be to override the Moose::Meta::Role::Application::ToClass class to add the preferred behavior. I've not dug into those bits directly so I'm not sure how easy or hard that would be. But I'd be happy to help, as would anybody on #moose on irc.perl.org.
Re:Mechanism vs Policy
Stevan on 2009-04-14T18:06:41
Okay, after much discussion on #moose (soon to be summarized on the mailing list) we will be adding a warning so that when a class method silently overrides the role method it will warn you that this is happening. In order to silence the warning you will have to explictly exclude the method from the role (see Moose::Cookbook::Roles::Recipe2 for information on this feature).
For more details, watch for my summary mail on the mailing list.
- Stevan
Re:Mechanism vs Policy
sartak on 2009-04-14T19:12:51
And here's the mailing list summary.
Re:Mechanism vs Policy
Ovid on 2009-04-15T02:28:15
You've made me a ridiculously happy man right now
:)
