Front page | perl.moose |
Postings from August 2013
Well they're relying on undefined behavior. Roles are defined to only compose *methods* not functions and relying on what they're doing a something that will "work" isn't guranteed to always work. Given a testcase and a patch that fixes it (but doesn't break anything else) I would argue Moose should adopt the more correct behavior of forbidding that.
I don't think that patch would be trivial to write, so I'm not sure you can rely upon that as an argument with whomever is adding this to your codebase at work.
If these roles are *only* doing this undefined "Exporter" like behavior, they're paying a performance penalty because the Moose code has to set up all the meta layers for Roles the exporting that Sub::Exporter (a dep from Moose) or Exporter itself (a core module) don't have to setup.
Beyond that, it's bad form and practice. It is the moral equivalent of using a hammer to put screws in the wall.
-Chris
On Thu, Aug 1, 2013 at 11:21 AM, Bill Moseley <moseley@hank.org> wrote:
> I just noticed this code again a work, and then found this email that I
> don't see every received a response.
> Can anyone provide their insight?
> On Sun, Sep 18, 2011 at 11:34 PM, Bill Moseley <moseley@hank.org> wrote:
>> I'm on vacation and have finally found wireless to check out the svn logs
>> back at work. I found a bunch of new Moose Roles created that are being
>> used like one might use Exporter. Here's an example:
>>
>> package App::Role::Date;
>> use strict;
>> use warnings;
>> use Moose::Role;
>> use DateTime;
>> use Carp;
>>
>>
>> sub date_is_in_future {
>> my ( $test_date ) = @_;
>>
>> croak 'first arg to date_is_in_future must be DateTime'
>> unless ref( $test_date ) eq 'DateTime';
>>
>> my $now = DateTime->now();
>>
>> if ( $test_date >= $now ) {
>> return 1;
>> }
>>
>> return 0;
>> }
>>
>> Then in a number of classes (most not Moose-based to start with) that have
>> attributes that hold a DateTime there's code added like this:
>>
>> with 'App::Role::Date';
>>
>> ...
>> my $dt = $self->start_time;
>> if ( date_is_in_future( $dt ) ) {
>> ....
>> }
>>
>> Perhaps I'm being pedantic, but that makes my skin crawl. For one thing
>> it's using a Role like Exporter. And "date_is_in_future" seems like it
>> should be a method on the DateTime object (although seems "$dt >
>> DateTime->now" is just as easy to write.) And, of course, the above will
>> fail on DateTime subclasses. Not to mention returning 0 or 1 gives me the
>> willies.
>>
>> Are there other more Moose-specific reasons that this code is problematic
>> that I can point out?
>>
>>
>>
>> --
>> Bill Moseley
>> moseley@hank.org
>>
> --
> Bill Moseley
> moseley@hank.org
Thread Previous
|
Thread Next