# New Ticket Created by  James Keenan 
# Please include the string:  [perl #44957]
# in the subject line of all future correspondence about this issue. 
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=44957 >


Parrot::Configure::Data currently holds two methods, dump() and  
dump_temp() which do virtually the same thing.  Here's the POD for  
dump():

=item C<dump()>

Provides a L<Data::Dumper> serialized string of the objects key/value  
pairs
suitable for being C<eval>ed.  The variable name of the structure is
C<PConfig>.

Accepts no arguments and returns a string.


The reason why dump() accepts no arguments is that it is hard-coded  
with two values:  the element in the Parrot::Configure object which  
is being dumped ('c') and the name for the structure once dumped  
(PConfig).

     Data::Dumper->new( [ $self->{c} ], ['*PConfig'] )->Dump();

dump_temp() does the same thing, except with two different,  
internally hard-coded values.

     Data::Dumper->new( [ $self->{c_temp} ], ['*PConfig_Temp'] )->Dump 
();

Hard-coding of arguments is bad.  Repeated code is a mistake.  Let's  
refactor Parrot::Configure::Data::dump() to require arguments and  
thereby gain flexibility.  In an upcoming patch, I will make use of  
dumps of elements of the Parrot::Configure object's data structure  
other than 'c' and 'c_temp' for reconfiguration purposes.

The patch attached eliminates dump_temp() and refactors dump() to  
take two arguments.  It then modifies the two files where dump() and  
dump_temp() are currently used:  config/gen/config_pm.pm and t/ 
configure/data.t.  (In a separate patch I am proposing renaming  
data.t, but we can take care of that later.)

See the datadump/ branch for reference.

Thank you very much.
kid51
Index: lib/Parrot/Configure/Data.pm
===================================================================
--- lib/Parrot/Configure/Data.pm        (revision 20860)
+++ lib/Parrot/Configure/Data.pm        (working copy)
@@ -16,7 +16,7 @@
     $data->set($key1 => $value1, $key2 => $value2);
     $data->add($delimiter, $key1 => $value1, $key2 => $value2);
     my @keys = $data->keys;
-    my $serialized = $data->dump;
+    my $serialized = $data->dump(q{c}, q{*PConfig});
     $data->clean;
     $data->settrigger($key, $trigger, $cb);
     $data->gettriggers($key);
@@ -217,54 +217,46 @@
 =item C<dump()>
 
 Provides a L<Data::Dumper> serialized string of the objects key/value pairs
-suitable for being C<eval>ed.  The variable name of the structure is
-C<PConfig>.
+suitable for being C<eval>ed.  
 
-Accepts no arguments and returns a string.
+Takes two arguments:
 
-=cut
+=over 4
 
-# Data::Dumper supports Sortkeys since 2.12
-# older versions will work but obviously not sorted
-{
-    if ( defined eval { Data::Dumper->can('Sortkeys') } ) {
-        *dump = sub {
-            my $self = shift;
-            Data::Dumper->new( [ $self->{c} ], ['*PConfig'] 
)->Sortkeys(1)->Dump();
-        };
-    }
-    else {
-        *dump = sub {
-            my $self = shift;
-            Data::Dumper->new( [ $self->{c} ], ['*PConfig'] )->Dump();
-        };
-    }
-}
+=item 1
 
-=item C<dump_temp()>
+Key in Parrot::Configure object's data structure which is being dumped.
 
-Provides a L<Data::Dumper> serialized string of the objects key/value pairs
-suitable for being C<eval>ed.  The variable name of the structure is
-C<PConfig_Temp>.
+=item 2
 
-Accepts no arguments and returns a string.
+Name of the dumped structure.
 
+=back
+
+Example:
+
+    $conf->data->dump(q{c}, q{*PConfig});
+    $conf->data->dump(q{c_temp}, q{*PConfig_Temp});
+
+Returns a string.
+
 =cut
 
 # Data::Dumper supports Sortkeys since 2.12
 # older versions will work but obviously not sorted
 {
-
-    if ( defined eval { Data::Dumper->can( 'Sortkeys' ) } ) {
-        *dump_temp = sub {
+    if ( defined eval { Data::Dumper->can('Sortkeys') } ) {
+        *dump = sub {
             my $self = shift;
-            Data::Dumper->new( [ $self->{c_temp} ], ['*PConfig_Temp'] 
)->Sortkeys(1)->Dump();
+            my ($key, $structure) = @_;
+            Data::Dumper->new( [ $self->{$key} ], [$structure] 
)->Sortkeys(1)->Dump();
         };
     }
     else {
-        *dump_temp = sub {
+        *dump = sub {
             my $self = shift;
-            Data::Dumper->new( [ $self->{c_temp} ], ['*PConfig_Temp'] 
)->Dump();
+            my ($key, $structure) = @_;
+            Data::Dumper->new( [ $self->{$key} ], [$structure] )->Dump();
         };
     }
 }
Index: t/configure/data.t
===================================================================
--- t/configure/data.t  (revision 20860)
+++ t/configure/data.t  (working copy)
@@ -164,7 +164,7 @@
 {
     my $pcd = Parrot::Configure::Data->new;
 
-    my $data = $pcd->dump;
+    my $data = $pcd->dump(q{c}, q{*PConfig});
 
     like( $data, qr/\%PConfig = \(\);/, "->dump() returns nothing if no keys 
are set" );
 }
@@ -178,7 +178,7 @@
         'b' => 2,
         'c' => 3,
     );
-    my $data = $pcd->dump;
+    my $data = $pcd->dump(q{c}, q{*PConfig});
 
     like(
         $data,
Index: config/gen/config_pm.pm
===================================================================
--- config/gen/config_pm.pm     (revision 20860)
+++ config/gen/config_pm.pm     (working copy)
@@ -51,8 +51,8 @@
     print {$OUT} "# Generated by config/gen/config_pm.pm\n";
 
     while (<$IN>) {
-        s/<<PCONFIG>>/$conf->data->dump()/e;
-        s/<<PCONFIGTEMP>>/$conf->data->dump_temp()/e;
+        s/<<PCONFIG>>/$conf->data->dump(q{c}, q{*PConfig})/e;
+        s/<<PCONFIGTEMP>>/$conf->data->dump(q{c_temp}, q{*PConfig_Temp})/e;
         print {$OUT} $_;
     }
 

Reply via email to