Skip to content

Instantly share code, notes, and snippets.

@nrdvana
Last active February 17, 2020 04:34
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save nrdvana/6c0a688b73cc968a3e4851a050cd9411 to your computer and use it in GitHub Desktop.
Save nrdvana/6c0a688b73cc968a3e4851a050cd9411 to your computer and use it in GitHub Desktop.

My Nitpicks with the Design of Moo

Verbosity

The attribute declaraions can get a little bit verbose. Moo(se) isn't bad, but it seems like we ought to be able to do a little better...

has foo => ( is => 'rw', isa => Type, lazy_build => 1, trigger => 1 );

Maybe:

has foo => 'rw,lazy,trigger', isa => Type;

In other words, put all the booleans into one initial string, and stop writing "is".

Constructor Coercion

Moo and Moose have a "coerce" feature. I like the concept of coercions, but the coderef only gets called with the input data, not with any access to the object. Sometimes the object is a factory of sorts, and the attribute is describing what thing to create, and the coercion would need access to the object.

The way I have solved this in the past is to store the constructor argument in a hidden attribute and then lazy-build the main attribute.

has _window => ( is => 'ro', init_arg => 'window' );
has window  => ( is => 'lazy', init_arg => undef );
sub _build_window($self) { $self->windowing_toolkit->create_window( $self->_window ) }

There might not be a fix for this, though, because coercion happens when the object is being initialized and it might not be safe to call methods yet. This is the problem that lazy-building solves, as in the example above.

There is probably still some way to streamline the example above. Wouldn't it be neat if _builder methods would receive a second argument of the value passed to the constructor? Then it could either coerce the second argument, or fall back to the default? And maybe the 'default' could be passed through the coercion as well, then subclasses could change the default using the "friendly" pre-coercion notation. And then, maybe taking the place of a trigger, the coercion could be called again but with an extra argument of the previous value?

has window => 'rw,lazy,coerced', default => sub { +{ width => 640, height => 480 } };
sub _coerce_window {
  my ($self, $new_value, $previous_value)= @_;
  # If running from constructor, @_ == 2 and $new_value is defined by new args, or by the 'default'
  # If running from a mutable method access, @_ == 3
  return $new_value->isa('Window')? $new_value : $self->windowing_toolkit->create_window( $new_value );
}

Can't describe "read/write but call my code if written"

I try to make objects immutable when possible, avoiding this problem, but for mutable ones I often want to enforce some dynamic condition before an attribute gets written.

sub attr1 {
  my ($self, $new_val)= @_;
  if (@_ == 1) {
    return $self->{attr1};
  }
  else {
    $state_can_write_attr1{ $self->state }
      or croak "Can't write attr1 after state Foo";
    $self->is_compatible( $new_val, $self->attr2 )
      or croak "Cannot set attr1 to $new_val when attr2 is ".$self->attr2;
    return $self->{attr1}= $new_val;
  }
}

Moo provides trigger, but that runs after the attribute has been set, leaving the object in an invalid state. Moo allows us to set writer, but that changes the public API. I could change reader => 'get_attr1' and writer => 'set_attr1' and then wrap them myself with sub attr1 { my $self= shift; @_? $self->set_attr1(@_) : $self->get_attr1(@_); } but this is annoying boilerplate and not as efficient, and defeats optimizations like Class::XSAccessor.

My proposal for the _coerce function above would solve this problem in about as little boilerplate as possible.

has attr1 => 'rw,coerce';
sub _coerce_attr1 {
  my ($self, $new_value, $previous_value)= @_;
  $state_can_write_attr1{ $self->state }
    or croak "Can't write attr1 after state Foo";
  $self->is_compatible( $new_val, $self->attr2 )
    or croak "Cannot set attr1 to $new_val when attr2 is ".$self->attr2;
  return $new_value;
}

Unused Constructor Arguments (i.e. typos)

I think there should be a built-in system for declaring how Moo should treat unused arguments. I have encountered use cases for each of three options:

  • Ignore them (same as now)
  • Warn about them
  • Die

Since all three have valid uses, I think there needs to be some sort of configuration knob to specify options for the constructor. Maybe Moo->constructor_opts(__PACKAGE__, ...) or something, to avoid adding another keyword.

XS Speed Boost

Moo(se) uses Class::XSAccessor for a speed boost, but people often don't know to install it. I think there should be a core module that has lots and lots of accessor and constructor patterns (as in, every popular option seen on CPAN, not just Moo(se) options) available so that all object systems (like Mojo, etc) can take advantage. The object systems should be able to inspect the metadata of attributes to recognize whether an attribute or constructor fully matches one of the available patterns, and then substitute the XS version.

If people can come to a concensus on a single API for the "slots metadata" like Steven Little was proposing, then maybe the XS accessor/ctor generator module would just take a list of slot definitions and return a list of XS subs, solving the problem for all the object system authors.

Role requirements

Saying requires "foo"; wants the method to be declared at the moment the role is consumed. Sometimes it will be supplied by a later role, or by some on-the-fly code in the body of the module. It would be nice if this was implicitly delayed until the end of the class declaration.

Moose has a hook for "end of declaration" in the form of the __PACKAGE__->meta->make_immutable but I really hate this quantity of boilerplate. I think the best solution is to use something like B::Hooks::EndOfScope. If that were a Perl core feature (an official hook/block for the end of current scope) it would be a benefit to everyone. Then, the object system could make use of it for things like checking consistency of roles.

BUILDARGS

I don't like the BUILDARGS system. I don't have any good ideas for a replacement though. Usually what I do is just leave 'new' as-is and then create a new method name for the alternate constructor, like CLASS->new_from_list(1, 2, 3, 4) or CLASS->coerce($other_thing). Maybe this is the best solution. Maybe a better way would be to let the package author decide what the Moo constructor should be named?

use Moo ctor_name => '_new';
sub new {
   ... # whatever I want
   Moo->call_constructor($class, \%built_args );
}

Method _new would become the Moo constructor interface, and new would be left as the public-facing method with whatever API the author liked best. The call_constructor method of Moo would give people a way to call the Moo-style constructor no matter what name a subclass had chosen.

FOREIGNBUILDARGS

I also don't like this one, but I've only used it once. Usually I've found it better to wrap a foreign object than to subclass it with an incompatible framework.

I feel like whatever was happening in BUILDARGS should not need to be repeated, and that Moo should do the job of extracting out some arguments before handing me the half-done work so that I can call some other constructor, then merge the object with Moo slots at the end. But, building on the idea above where Moo lets me write my own new method, I would just need the ability to call pieces of the Moo constructor. (specifically the piece that would run after the call to FOREIGNBUILDARGS and the foreign constructor)

sub new {
  my %attrs= ...; # do whatever needed to extract this subclass's arguments from the input
  my $self= $class->next::method(@_);
  Moo->augment_foreign_object($self, %attrs);
}
@Grinnz
Copy link

Grinnz commented Oct 17, 2019

FYI, default takes non-ref scalars without having to get them from a sub already. builder needs a sub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment