From bfd9267fac4932a0a4c86167fe9183e2c18a9b07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Fri, 26 Feb 2016 01:20:06 +0100 Subject: [PATCH 1/2] improve must_be implementation: spec transformed into a closure The __is_a sub is replaced by a closure built from the $must_be specification. --- lib/Data/OptList.pm | 46 +++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/lib/Data/OptList.pm b/lib/Data/OptList.pm index 762cafe..22e5837 100644 --- a/lib/Data/OptList.pm +++ b/lib/Data/OptList.pm @@ -131,23 +131,11 @@ BEGIN { ); } -sub __is_a { - my ($got, $expected) = @_; - - return List::Util::first { __is_a($got, $_) } @$expected if ref $expected; - - return defined ( - exists($test_for{$expected}) - ? $test_for{$expected}->($got) - : Params::Util::_INSTANCE($got, $expected) ## no critic - ); -} - sub mkopt { my ($opt_list) = shift; my ($moniker, $require_unique, $must_be); # the old positional args - my $name_test; + my ($name_test, $is_a); if (@_) { if (@_ == 1 and Params::Util::_HASHLIKE($_[0])) { @@ -156,9 +144,35 @@ sub mkopt { } else { ($moniker, $require_unique, $must_be) = @_; } - } - $moniker = 'unnamed' unless defined $moniker; + # Transform the $must_be specification into a closure $is_a + # that will check if a value matches the spec + + # Array with single element => scalar + # Array with no elements => undef + if (ref($must_be) && @$must_be <= 1) { + $must_be = $must_be->[0] + } + if (defined $must_be) { + if (ref $must_be) { + # $must_be is an array with at least 2 elements + my @checks = map { + my $class = $_; + $test_for{$_} + || sub { $_[1] = $class; goto \&Params::Util::_INSTANCE } + } @$must_be; + $is_a = sub { + my $value = $_[0]; + List::Util::first { defined($_->($value)) } @checks + }; + } else { + $is_a = $test_for{$must_be} + || sub { $_[1] = $must_be; goto \&Params::Util::_INSTANCE } + } + + $moniker = 'unnamed' unless defined $moniker; + } + } return [] unless $opt_list; @@ -185,7 +199,7 @@ sub mkopt { $i++ } elsif (! $name_test->($opt_list->[$i+1])) { $value = $opt_list->[++$i]; - if ($must_be && !__is_a($value, $must_be)) { + if ($is_a && !$is_a->($value)) { my $ref = ref $value; Carp::croak "$ref-ref values are not valid in $moniker opt list"; } From b0e7634104153d1ae64e10ab3cfcd1a01c076bcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Sat, 27 Feb 2016 00:25:20 +0100 Subject: [PATCH 2/2] Simplify $must_be compiler The code is slightly slower for the scalar $must_be case (as we now transform that case in the array case with a single element), but simpler to read/maintain as closures are built in a single place. --- lib/Data/OptList.pm | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/lib/Data/OptList.pm b/lib/Data/OptList.pm index 22e5837..edb4492 100644 --- a/lib/Data/OptList.pm +++ b/lib/Data/OptList.pm @@ -148,27 +148,20 @@ sub mkopt { # Transform the $must_be specification into a closure $is_a # that will check if a value matches the spec - # Array with single element => scalar - # Array with no elements => undef - if (ref($must_be) && @$must_be <= 1) { - $must_be = $must_be->[0] - } if (defined $must_be) { - if (ref $must_be) { - # $must_be is an array with at least 2 elements - my @checks = map { - my $class = $_; - $test_for{$_} - || sub { $_[1] = $class; goto \&Params::Util::_INSTANCE } - } @$must_be; - $is_a = sub { - my $value = $_[0]; - List::Util::first { defined($_->($value)) } @checks - }; - } else { - $is_a = $test_for{$must_be} - || sub { $_[1] = $must_be; goto \&Params::Util::_INSTANCE } - } + $must_be = [ $must_be ] unless ref $must_be; + my @checks = map { + my $class = $_; + $test_for{$_} + || sub { $_[1] = $class; goto \&Params::Util::_INSTANCE } + } @$must_be; + + $is_a = (@checks == 1) + ? $checks[0] + : sub { + my $value = $_[0]; + List::Util::first { defined($_->($value)) } @checks + }; $moniker = 'unnamed' unless defined $moniker; }