[Tallahassee-pm] Perl, a better way to do this with OOP

[Tillman, James]

Philip:  Are you asking how to better solve this problem through OOP, or are
you asking for ways to improve your use of this object-oriented module that
someone else wrote?  
 
If you're asking how to solve your problems through OOP, I would recommend
creating a wrapper object that takes the value names you dislike and maps
them to names you DO like.  This will encapsulate the offending code and
shield the rest of your project from it.  There is currently no OOP code in
the sample you provided (other than your use of the 3rd party's object) so
I'm not sure what your comfort level is with Perl OOP.  If you do want more
help on this, I can certainly provide it, but it will be a little more
in-depth, so I'll let you indicate that you need this before I get off on a
long tangent here.
 
If you simply want to improve your own use of the object in question by
improving the code you provided, I would recommend that you look into the
keys() and values() functions, which will help you avoid having to enter
your list of "lousy_names" 2 times.  Try something like this:
 
%RecordParams = ( 

    "lousy_name_1" => "Meaningful name 1",

    "lousy_name_2" => "Meaningful name 2",

    "lousy_name_3" => "Meaningful name 3",

);

 

# Perl requires lowercase "sub", not "Sub"!

sub printRecordDetails {

    my $record = shift;

    # keys() will return the array ( "lousy_name_1", "lousy_name_2",
"lousy_name_3" ) based on what

    #  you have assigned as keys for the %RecordParams hash.  values() would
return

    #  the array ("Meaningful Name 1", etc... )

    foreach my $line ( keys(%RecordParams) ) {

        print $RecordParams{$line} . " =  " . $record->param($line) . "\n";

    }

}

 

Another thing you might try is ditching the subroutine and using:

 

for my $record  ( keys(%RecordParams) ) {

    foreach my $line ( @RecordOrder) {

        print $RecordParams{$line} . " =  " . $record->param($line) . "\n";

    }

}

 

That's much more readable and succinct.  It actually cuts your script down
to around 12 lines (not counting comments and white space).

 

As far as I can tell, there's no multidimensional array being used by your
3rd party object.  In fact, Perl doesn't support them.  It only supports
arrays of arrays, which can be used to solve similar problems, anyhow, and
can often appear to be multi-dimensional.  Fortunately, it doesn't look like
you'll have to deal with that kind of complexity here.
 
What you get by accessing $object->{"Record"} is a single dimensional array
of objects.  Each of these objects seems to have a "param" method that
allows you get the value of a single field of the record.  I agree this
could have been much better designed.
 
I would prefer to see something like this:
 
while ( my $record = $object->getNext() ) {
        foreach my $line ( @RecordOrder ) { 

            print $RecordParams{$line} . " =  " . $record->param($line) .
"\n";

        }

 
This creates a getNext() method that returns the appropriate record object
when called and moves an internal pointer to the next record in the object's
array list.  Another option would have been:
 
while ($object->moveNext() ) {
        foreach my $line ( @RecordOrder ) { 

            print $RecordParams{$line} . " =  " . $object->param($line) .
"\n";

        }

 
This form removes the need for the "$record" object to be provided.  You
just use the "$object" object and that's all.
 
Requiring consumers of your OOP code to use the $object->{"Record"} form of
accessing your object's data is catastrophically bad form unless you're both
the author of the OOP code and the only consumer of it.  This is somewhat
equivalent to using public variables in object-oriented Java, C++, or VB
code and expecting your consumers to use them.  It's a recipe for disaster.
 
jpt
 

 -----Original Message-----
From: Phillip Tyre [mailto:phillip.tyre at fcul.com]
Sent: Friday, December 05, 2003 10:55 AM
To: tallahassee-pm at mail.pm.org
Subject: [Tallahassee-pm] Perl, a better way to do this with OOP



It's not that I'm not a huge fan of the object oriented parts of Perl, I'm
just not that versed in it. That said, I find my self working on a for fun
project that uses a lot of OOP. I'm trying to get up to speed, and I have
something that almost works, but it's not elegant. It's also not very
readable!

 

I started with a program someone else had written to walk through the object
and print out the information.

Abstracting it a bit, it looks like this:

 

            I have a library that someone else wrote that unpacks a binary
file into an object.

            # I get an object containing the file contents with:

            my $object = Module:Method->new_object_from_file(file);

 

            # The names used by the object are not the best in the world, so
we have a line like this:

 

            %RecordParams = (

                                     "lousy_name_1" => "Meaningful name 1",

                                    "lousy_name_2" => "Meaningful name 2",

                                    "lousy_name_3" => "Meaningful name 3",

                                    );

 

            # Then we create a new array with:

            @RecordOrder = qw ( lousy_name_1 lousy_name_2 lousy_name_3 );

            

            T# hen we have a subroutine to print out the info for a record:

            Sub printRecordDetails {

                        my $record = shift;

                        foreach my $line ( @RecordOrder) {

                                    print $RecordParams{$line} . " =  " .
$record->param($line) . "\n";

                        }

            }

 

            # Use a method of the object to return a (multidimensional?)
array of all the records.

            my @records = @{$object->{"Record"}};

 

            # Then we call the subroutine in a loop for each record

            for my $record  ( @records ) {

                        printRecordDetails( $record );

 

            # end

 

And that's it, now keep in mind this is an example script provided by
someone else to make use of the library. It just doesn't seem like the best
way to do things, but I suppose I don't know enough about OOP to know what
questions to ask to find answers to make it better.

            

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.pm.org/pipermail/tallahassee-pm/attachments/20031208/a3987ad6/attachment.htm