All the Perl that's Practical to Extract and Report

Spot the Bug!

Ovid on 2009-06-03T14:57:51

Today I spent a couple of hours tracking this bug down. The bug is mine, but relies on what I think is rather counter-intuitive behavior in Moose (due to how Class::MOP works). I trawled through a lot of source code before I could replicate a simple test case. I'm not sure why Moose behaves this way and from what I can tell, it's not documented in Moose, but implied via the Class::MOP::Attribute documentation.

#!/usr/bin/env perl -l

{
    package My::Base;
    use Moose;
    has some_method => ( default => 'foo' );
}
{
    package My::Class;
    use Moose;
    extends 'My::Base';
}
print My::Class->new->some_method;

If you don't know Moose, you won't see the bug. In fact, I'd wager that many who are comfortable with Moose won't see the bug right away, if at all. You'll see the problem if you run the code (I'm on the latest version of Moose), but the fix won't be apparent unless you understand why the bug occurs.

explanation?

depesz on 2009-06-03T15:05:33

I definitely don't know Moose well enough, and I fail to see why it doesn't work.

Care to provide some explanation?

Re:explanation?

rjbs on 2009-06-03T15:08:25

The attribute is not declared rw/ro or given a reader, so no method is created.

Re:explanation?

depesz on 2009-06-03T15:12:29

Well, OK, but then the example provided is not minimal - My::Class is actually not relevant. The problem would display itself also if it was called directly on My::Base.

Re:explanation?

Ovid on 2009-06-03T15:51:59

And it does display itself if called directly on My::Base. I represented it as above because that's how it manifested in our code.

So you intentionally made it misleading?

perigrin on 2009-06-03T18:27:06

Is this because you secretly hate Moose?

Re:So you intentionally made it misleading?

Ovid on 2009-06-03T20:30:15

I love Moose. I think it's one of the most fantastic things to happen in Perl in a long time. It's just from time to time I would do things differently, but that's OK. It's not possible to have a project this significant in which everyone will agree with every decision.

Why...?

lbr on 2009-06-03T15:54:34

...did you call it 'some_method'. has is not sub. It's a nice red herring though, throwing me off completely till I saw rjbs's answer. If it'd been called "some_field", it'd been obvious.

rjbs has explained what went wrong, but I'm curious why you'd expect it to behave differently. The behaviour is exactly as documented in Moose's section on is => 'rw'|'ro'.

Re:Why...?

Ovid on 2009-06-03T16:14:50

Sorry about that. I just hastily threw together an example. But what's the value of allowing me to declare an attribute without an accessor or mutator? I'm sure there's a reason. I just don't see it.

Re:Why...?

phaylon on 2009-06-03T16:33:18

The slot is still accessible via the MOP, which means other accessor-like methods can access it. For example, with MooseX-AttributeHelpers you could have an ArrayRef container providing push and pop methods without providing an API to directly set the array reference value.

Re:Why...?

Aristotle on 2009-06-03T17:49:00

It would be nice to have to explicitly request that, though? Instead of it being the silent default. Or else there might be has_ro and has_rw keywords that one could use for the bulk of one’s declarations, in which case it’d be immediately obvious when something uncommon is going on.

Re:Why...?

jjn1056 on 2009-06-03T17:58:23

You could do this with an attribute trait, but putting "is=>'ro'" is not particularly onerous and it reinforces the idea that an attribute is not just a fancy getter/setter sugar code, but is fundementally more powerful.

At this point I doubt this is going to change in Moose, although MooseX::Declare might be open to new ideas (not sure what the authors of that have in mind over the long term.).

Re:Why...?

Stevan on 2009-06-03T18:38:51

At this point I doubt this is going to change in Moose, although MooseX::Declare might be open to new ideas.

MooseX::Declare is part of the overall Moose project, it will almost certainly not deviate too far from what Moose does on key issues like this. However which might be nice is to add support for the Perl-6 style of doing this, like so:

class Foo is rw {
   has bar;
   has baz;
}

Here all of Foos attributes will be (is => rw) automagically. I think the key issue here is that Moose will not and should not do anything you don't ask it to do. DWIMery is very VERY slippery slope, too much of it is really bad and too little gets you tedious code (see also Vanilla Perl OOP). Moose tries to strike a balance and not be too knee-jerk about adding support for DWIMery.

- Stevan

Re:Why...?

Ovid on 2009-06-03T20:50:07

My only suggestion is that the default be (is => 'ro') for reasons that I think are probably obvious to you. That being said, I accept an 'rw' default as most OO programmers are going to expect that an not agree that 'ro' is so important.

Re:Why...?

perigrin on 2009-06-03T22:17:13

class Foo is ro {
      has bar;
      has baz;
}

# or in Plain Moose

{
package Foo;
use Moose -traits => ['ReadOnly'];
use namespace::autoclean;

has bar;
has baz;

__PACKAGE__->meta->make_immutable;
}

Re:Why...?

Yanick on 2009-06-03T16:36:25

There are a few reasons I can imagine, and several I'm fairly sure I'm too afraid to consider. :-)

One reason would be because one wants to explicitly set the 'reader' and the 'writer'.

has babelfish => (
    writer => 'schrieb_fish',
    reader => 'lit_fish',
);

But that doesn't explain why there is no warning if there is no reader/writer assigned at all. But that could happen because one want to create those sometime during runtime. Wacky, but I can imagine a few motivations to do that.

Or it could be that the attribute is used for a trait that does stuff not requiring the class to set/access its value (I can't think of a concrete example, but I'm sure one could think of one, provided with enough chemical stimulants):

    # we don't know who they are,
    # or what they do, but they do
    # control this class, somehow...
has illuminati => (
    traits => [ qw/ Secret::Organization / ],
);

Re:Why...?

perigrin on 2009-06-03T18:19:53

role Pet::Sounds {
    method make_noise { say $self->noise }
}

class MyHouse {
    has pet => (
        does => 'Pet::Sounds',
        required => 1,
        handles =>  'Pet::Sounds',
    );
}
MyHouse->new(pet => Beetle->new())->make_noise; # I want pet provided on creation but never accessed directly after that.

Re:Why...?

Stevan on 2009-06-03T18:26:53

The answer to "why" is simply "because you didn't ask for it". DWIMery is good up until it gets in your way, then it's annoying and difficult to work around. Here is another example (along with perigrin's excellent one) on why you might not want an accessor.

has foo => ( trigger => sub { my ($self, $value) = @_; $self->bar( $value + 2 ) } );
has bar => ( is => 'rw', ... );

Here the side-effectual trigger is used to set the value of bar. Sure, seems kind of esoteric, but it is a valid use case.

- Stevan

This is not a bug

Stevan on 2009-06-03T18:22:40

Ovid,

Your question was answered on the mailing list, it is not a bug and this issue has been discussed many times before over the last 3 years and we always come back to the same conclusion that a default for is is too much DWIMery.

- Stevan

Re:This is not a bug

Ovid on 2009-06-03T21:22:25

Thanks Stevan. No worries. I personally accept some DWIMery for what I envision as the common use case and accept that this sometimes make the syntax a bit more difficult for other cases, but at the end of the day, it's no big deal. Now that I understand this, I can deal with it, though a better error message might help (though at the cost of tracking this information, I've no idea if the net impact for this use case is beneficial).

My apologies for raising an issue that's been raised before. I googled quite a bit for this, but never found the right combination of keywords.

Re:This is not a bug

Stevan on 2009-06-04T00:02:07

I personally accept some DWIMery for what I envision as the common use case and accept that this sometimes make the syntax a bit more difficult for other cases, but at the end of the day, it's no big deal.

I understand your issue, but the number of possible combinations available for has is huge and when you start introducing default values for any of them you are heading down a very slippery slope. As you said in the comment above, you would prefer ro, but some people would expect rw, while still others would rather we support PBP-style accessors and then there is the semi-affordance crowd, and the public reader/private writer crowd, the list can go on and on. In my mind the only solution to all these differing (and equally valid) viewpoints is to favor none of them but allow all of them (which is exactly what Moose does). I think this embodies the spirit of TIMTOWTDI and is therefore (IMO at least) the most "perlish".

Now that I understand this, I can deal with it, though a better error message might help (though at the cost of tracking this information, I've no idea if the net impact for this use case is beneficial).

The problem with an error message is the same as trying to decide which default to use. Someone wins and someone looses. If I throw a warning when there is no value for 'is', then someone who has a legit use would have to suppress the error. Personally i think that the best solution here is Perl::Critic (same as the role method issue you brought up). Perl::Critic is entirely optional, very configurable and does not impose any runtime or compile-time penalties.

>My apologies for raising an issue that's been raised before. I googled quite a bit for this, but never found the right combination of keywords.

Unfortunately most of this discussion has actually taken place on #moose and therefore is not easily google-able.

- Stevan

Re:This is not a bug

Ovid on 2009-06-04T06:54:37

The idea of a better error message would actually be to trap the error and see if there was an attribute up the tree which didn't have any accessor defined for it and warn on that. That being said, it seems like it might be too much work for too little benefit.