Chad,

Trigger is called by the accessor, which is why the infinite recursion. I wouldn't recommend trigger for this kind of thing, personally I tend to use trigger for things like syncing two different attributes and not to clean data.

Based on what I am seeing here, I think coercion is your best approach (which is the conclusion you came to as well) and for several reasons.

1) It is by far the most re-usable solution since any other attribute that needs a clean string can just use the type 2) It is also the most extendable since you can simply subtype the CleanStr type if you want to have a clean string that is also HTML encoded 3) It is *exactly* the problem that the type-constraint/type-coercion system is meant to solve, the prevention of bad data and simple ways to convert that bad data into good data.

- Stevan

On Feb 16, 2009, at 1:09 PM, Chad Davis wrote:
I'm re-posting this to the Moose list. Hopefully some of the Moose
experts can offer their insight.

The following is about cleaning-up attributes, before allowing them to
be set, e.g. HTML encoding or removing invalid characters from a
string input. The question is which Moose approach is best to
accomplish this: before, after, around, trigger, BUILD, or coerce.


At a higher level, I wasn't sure when it's more appropriate to use
before/after/around vs. triggers.
E.g. before/after/around are not called for attributes set in the
constructor. In that case, trigger does work, though. However, for doing cleanup of attribute values before allowing them to be set (e.g. HTML encoding a string), trigger does not seem to get access to @_ So, if I use trigger to recall $self->myattribute(), I'll have infinite recursion. In
the
end, I found coercion to be the best (only?) way. This kind of
attribute-cleanup wrapper might be something for the cookbook.

I'm lost here. Triggers _are_ passed the value of the attribute set in the
constructor, or at least that's what the docs say ;)

Triggers don't have access to _all_ of the arguments passed to the
constructor, though. For that, use BUILD or BUILDARGS.


I admit that trigger does get @_, like you said, but if I call the
accessor from the trigger I get infinite recursion:

package Cleaner;
use Moose;
has 'myattr' => (
  is => 'rw',
  isa => 'Str',
#     trigger => \&triggercleaner,
  );
sub triggercleaner {
  my ($self, $arg) = @_;
  $arg =~ s/\s//g;
  $self->myattr($arg);   # <--- infinite recursion
}

'Around' would be the ideal solution:

around 'myattr' => \&aroundcleaner;
sub aroundcleaner {
  my ($orig, $self, $val) = @_;
  return $self->$orig() unless $val;
  $val =~ s/\s//g;
  return $self->$orig($val);
}

Except that these two bits of code produce different results,
depending on where myattr is set. So I cannot enforce that the value
is always clean.

my $x = new Cleaner(myattr=>"around not called from constructor");
print "Spaces still there:", $x->myattr, "\n";

# But then calling this does call aroundcleaner()
$x->myattr($x->myattr);
print "Spaces removed ", $x->myattr, "\n";

So, I need a BUILD, because the constructor is a special case:

sub BUILD {
  my ($self) = @_;
  # either this:
  aroundcleaner(\&myattr, $self, $self->myattr);
  # or this will work:
  $self->myattr($self->myattr);
}

But that needs to be done for any further attributes that need
cleaning. If this were classic OO, it would just be:

sub myattr {
  my ($self, $val) = @_;
  return $self->{'myattr'} unless defined $val;
  $val =~ s/\s//g;
  return $self->{'myattr'} = $val;
}

Which is why I'm slightly perplexed, because this seems to be the one
use case where Moose requires more code than classic OO.

I could just break the interface and do: $self->{'myattr'} = $val
inside the trigger, but none of us wants that.

I finally settled on coercion to do something like:

use Moose::Util::TypeConstraints;
subtype 'CleanStr' => as 'Str';
coerce 'CleanStr'
  => from 'Str'
  => via { $_ =~ s/\s//g; $_ };
has 'myattr' => (
  is => 'rw',
  isa => 'CleanStr',
  coerce => 1,
  );

But this also seems like a round-about solution, since I all I want is
to insert somewhere: s/\s//g

Is someone still following that might also have a good solution for
preventing unwanted values from getting into an attribute?

Cheers,
Chad

Reply via email to