Change the popn/shiftn arg checks to be positive instead of negative.

Insted of enumerating all the ways it could go wrong, tell the user how it should go right. For #149
evalEmpire · Nov 25, 2011 · 9f64b2e · 9f64b2e
1 parent 0c2d470
commit 9f64b2e
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 16 deletions.
diff --git a/lib/perl5i/2/ARRAY.pm b/lib/perl5i/2/ARRAY.pm
@@ -56,12 +56,10 @@ method grep($filter) {
 }
 
 method popn($times) {
-    Carp::croak("popn() takes a single argument, the number of elements to pop")
+    Carp::croak("popn() takes the number of elements to pop")
       unless defined $times;
-    Carp::croak("popn() takes a numerical argument")
+    Carp::croak("popn() takes a positive integer or zero")
-      unless $times->is_integer;
+      unless $times->is_integer && ($times->is_positive or $times == 0);
-    Carp::croak("popn() does not take negative arguments")
-      if $times < 0;
 
     # splice() will choke if you walk off the array, so rein it in
     $times = scalar(@$self) if ($times > scalar(@$self));
@@ -71,12 +69,10 @@ method popn($times) {
 }
 
 method shiftn($times) {
-    Carp::croak("shiftn() takes a single argument, the number of elements to pop")
+    Carp::croak("shiftn() takes the number of elements to shift")
       unless defined $times;
-    Carp::croak("shiftn() takes a numerical argument")
+    Carp::croak("shiftn() takes a positive integer or zero")
-      unless $times->is_integer;
+      unless $times->is_integer && ($times->is_positive or $times == 0);
-    Carp::croak("shiftn() does not take negative arguments")
-      if $times < 0;
 
     # splice() will choke if you walk off the array, so rein it in
     $times = scalar(@$self) if ($times > scalar(@$self));

diff --git a/t/popn.t b/t/popn.t
@@ -8,19 +8,19 @@ use Test::More;
 note "popn with no args"; {
     my @array = (1, 2, 3);
     throws_ok { @array->popn(); }
-      qr{^\Qpopn() takes a single argument, the number of elements to pop};
+      qr{^\Qpopn() takes the number of elements to pop at $0 line };
 }
 
 note "popn with negative arg"; {
     my @array = (1, 2, 3);
     throws_ok { @array->popn(-20); }
-      qr{^\Qpopn() does not take negative arguments at $0 line};
+      qr{^\Qpopn() takes a positive integer or zero at $0 line };
 }   
 
 note "popn with non-numerical argument"; {
     my @array = (1, 2, 3);
     throws_ok { @array->popn("rawr"); }
-    qr{^\Qpopn() takes a numerical argument};
+    qr{^\Qpopn() takes a positive integer or zero at $0 line };
 }
 
 note "popn with arg == 0"; {

diff --git a/t/shiftn.t b/t/shiftn.t
@@ -8,19 +8,19 @@ use Test::More;
 note "shiftn with no args"; {
     my @array = (1, 2, 3);
     throws_ok { @array->shiftn(); }
-      qr{^\Qshiftn() takes a single argument, the number of elements to pop};
+      qr{^\Qshiftn() takes the number of elements to shift at $0 line };
 }
 
 note "shiftn with negative arg"; {
     my @array = (1, 2, 3);
     throws_ok { @array->shiftn(-20); }
-      qr{^\Qshiftn() does not take negative arguments at $0 line};
+      qr{^\Qshiftn() takes a positive integer or zero at $0 line };
 }   
 
 note "shiftn with non-numerical argument"; {
     my @array = (1, 2, 3);
     throws_ok { @array->shiftn("meow"); }
-      qr{^\Qshiftn() takes a numerical argument};
+      qr{^\Qshiftn() takes a positive integer or zero at $0 line };
 }
 
 note "shiftn with arg == 0"; {