use keys instead of positioned parameters, cleanup

lib/Algorithm/Diff/Callback.pm

@@ -12,69 +12,45 @@ our $VERSION   = '0.03';
 our @EXPORT_OK = qw(diff_hashes diff_arrays);
 
 sub diff_hashes {
-    my ( $old, $new, $del_cb, $add_cb, $changed_cb ) = @_;
-    my @changed = ();
+    my ( $old, $new, %cbs ) = @_;
 
-    # check old and new hashes
     ref $old eq 'HASH' or croak 'Arg 1 must be hashref';
     ref $new eq 'HASH' or croak 'Arg 2 must be hashref';
 
-    # check callbacks
-    {
-        my $count = 3;
-        foreach ( $del_cb, $add_cb, $changed_cb ) {
-            if ( defined $_ ) {
-                ref $_ eq 'CODE' or croak "Arg $count must be coderef or undef";
-            }
-
-            $count++;
-        }
-    }
-
-    # start doing the work
-    foreach my $cell ( keys %{$new} ) {
-        if ( ! exists $old->{$cell} ) {
-            $add_cb and $add_cb->( $cell, $new->{$cell} );
+    my @changed = ();
+    foreach my $key ( keys %{$new} ) {
+        if ( ! exists $old->{$key} ) {
+            exists $cbs{'added'}
+                and $cbs{'added'}->( $key, $new->{$key} );
         } else {
-            push @changed, $cell;
+            push @changed, $key;
         }
     }
 
-    foreach my $cell ( keys %{$old} ) {
-        if ( ! exists $new->{$cell} ) {
-            $del_cb and $del_cb->( $cell, $old->{$cell} );
+    foreach my $key ( keys %{$old} ) {
+        if ( ! exists $new->{$key} ) {
+            exists $cbs{'deleted'}
+                and $cbs{'deleted'}->( $key, $old->{$key} );
         }
     }
 
-    foreach my $changed (@changed) {
-        my $before = $old->{$changed} || '';
-        my $after  = $new->{$changed} || '';
+    foreach my $key (@changed) {
+        my $before = $old->{$key} || '';
+        my $after  = $new->{$key} || '';
 
         if ( $before ne $after ) {
-            $changed_cb and $changed_cb->( $changed, $before, $after );
+            exists $cbs{'changed'}
+                and $cbs{'changed'}->( $key, $before, $after );
         }
     }
 }
 
 sub diff_arrays {
-    my ( $old, $new, $del_cb, $add_cb ) = @_;
+    my ( $old, $new, %cbs ) = @_;
 
-    # check old and new hashes
     ref $old eq 'ARRAY' or croak 'Arg 1 must be arrayref';
     ref $new eq 'ARRAY' or croak 'Arg 2 must be arrayref';
 
-    # check callbacks
-    {
-        my $count = 3;
-        foreach ( $del_cb, $add_cb ) {
-            if ( defined $_ ) {
-                ref $_ eq 'CODE' or croak "Arg $count must be coderef or undef";
-            }
-
-            $count++;
-        }
-    }
-
     # normalize arrays
     my @old = uniq sort @{$old};
     my @new = uniq sort @{$new};
@@ -86,9 +62,9 @@ sub diff_arrays {
             my ( $change, undef, $value ) = @{$changeset};
 
             if ( $change eq '+' ) {
-                $add_cb and $add_cb->($value);
+                exists $cbs{'added'} and $cbs{'added'}->($value);
             } elsif ( $change eq '-' ) {
-                $del_cb and $del_cb->($value);
+                exists $cbs{'deleted'} and $cbs{'deleted'}->($value);
             } else {
                 croak "Can't recognize change in changeset: '$change'";
             }

t/croak.t

@@ -16,7 +16,7 @@ my @old = qw( one two  );
 my @new = qw( one four );
 $|++;
 
-eval { diff_arrays( \@old, \@new, sub {}, sub {} ) };
+eval { diff_arrays( \@old, \@new, added => sub {}, deleted => sub {} ) };
 ok( $@, 'Caught error' );
 
 like( $@, qr/Can't recognize change in changeset\: '\*'/, 'Unknown change' );

t/diff_arrays.t

@@ -11,25 +11,35 @@ my @new = qw( john jim james jojo   jackie );
 
 diff_arrays(
     \@old, \@new,
-    sub {
+    deleted => sub {
         my $val = shift;
         is( $val, 'jarule', 'Goodbye jarule!' );
     },
-    sub {
+    added => sub {
         my $val = shift;
         is( $val, 'jojo', 'Hello jojo!');
     },
 );
 
 my $empty_list = 0;
-diff_arrays( [], [], sub { $empty_list++ }, sub { $empty_list++ } );
+diff_arrays(
+    [], [],
+    added => sub { $empty_list++ }, deleted => sub { $empty_list++ },
+);
 cmp_ok( $empty_list, '==', 0, 'Empty list does not get called' );
 
 my $no_change = 0;
-diff_arrays( \@old, \@old, sub { $no_change++ }, sub { $no_change++ } );
+diff_arrays(
+    \@old, \@old,
+    added => sub { $no_change++ }, deleted => sub { $no_change++ },
+);
 cmp_ok( $no_change, '==', 0, 'No change does not get called' );
 
 my $new_count = 0;
-diff_arrays( [], \@new, sub { $new_count-- }, sub { $new_count++ } );
+diff_arrays(
+    [],
+    \@new,
+    deleted => sub { $new_count-- }, added => sub { $new_count++ },
+);
 cmp_ok( $new_count, '==', scalar @new, 'New from scratch' );
 

t/diff_hashes.t

@@ -22,17 +22,17 @@ my %new = (
 
 diff_hashes(
     \%old, \%new,
-    sub {
+    deleted => sub {
         my ( $name, $val ) = @_;
         is( $name, 'band',      'Band was removed' );
         is( $val,  'Catharsis', 'Correct band'     );
     },
-    sub {
+    added => sub {
         my ( $name, $val ) = @_;
         is( $name, 'artist',          'Artist added'   );
         is( $val,  'Michael Jackson', 'Correct artist' );
     },
-    sub {
+    changed => sub {
         my ( $name, $before, $after ) = @_;
         is( $name,   'tvshow',          'Changing tv show'         );
         is( $before, 'Psych',           'Was Psych'                );
@@ -42,19 +42,21 @@ diff_hashes(
 
 my $empty_hash = 0;
 my $cb = sub { $empty_hash++ };
-diff_hashes( {}, {}, $cb, $cb, $cb );
+diff_hashes( {}, {}, deleted => $cb, added => $cb, changed => $cb );
 cmp_ok( $empty_hash, '==', 0, 'Empty hash does not get called' );
 
 my $no_change = 0;
 $cb = sub { $no_change++ };
-diff_hashes( \%old, \%old, $cb, $cb, $cb );
+diff_hashes( \%old, \%old, deleted => $cb, added => $cb, changed => $cb );
 cmp_ok( $no_change, '==', 0, 'No change does not get called' );
 
 my $new_count    = 0;
 my $change_count = 0;
 diff_hashes(
     {}, \%new,
-    sub { $new_count-- }, sub { $new_count++ }, sub { $change_count++ },
+    deleted => sub { $new_count--    },
+    added   => sub { $new_count++    },
+    changed => sub { $change_count++ },
 );
 
 cmp_ok( $new_count,    '==', scalar keys (%new), 'New from scratch' );