Help with understanding/using dispatch tables

Discussion in 'Perl Misc' started by Mothra, Apr 12, 2007.

  1. Mothra

    Mothra Guest

    Hello everyone,

    I have a perl script (see below) that uses dispatch tables to set values,
    however
    subs get_lat_N and get_lat_S are almost identical :-( this tells me thatt I
    am
    doing something wrong. How can I modify my dispatch table structure to avoid
    duplicate code?

    Thanks

    Mothra



    -------script below--------

    use HTTP::Request::Common;
    use POSIX qw(floor);

    #use Data::Dumper;
    use HTML::Form;
    use LWP::UserAgent;

    #use LWP::Debug qw(+);
    use strict;
    use warnings;

    my %dispatch_table_lat = (
    'N' => \&get_lat_N,
    'S' => \&get_lat_S
    );
    my %dispatch_table_lon = (
    'E' => \&get_long_E,
    'W' => \&get_long_W
    );

    our %location = ();

    #open (OUT, ">> d:/tmp/fetched_cities.txt") or die "Can't open file:$!\n";
    while (<DATA>) {

    /(\w+),\s+(\w+)\s+(\d+)\s+(\d+)\s+(\w)\s+(\d+)\s+(\d+)\s+(\w)/;

    $dispatch_table_lat{$5}->( $3, $4 );

    $dispatch_table_lon{$8}->( $6, $7 );

    $location{'zz1'} = floor( $location{'xx1'} / 15 );
    my ( $sunrise, $sunset ) = get_sunrise();

    print "$1 $2 sunrise: $sunrise sunset: $sunset\n";

    #print OUT "$1, $2 $3 $4 $5 $6 $7 $8 sunrise: $sunrise sunset:
    $sunset\n";

    }

    close OUT;

    sub get_lat_N {
    my ( $degree, $minute ) = @_;
    $location{'yy0'} = '1';
    $location{'yy1'} = $degree;
    $location{'yy2'} = $minute;
    return;
    }

    sub get_lat_S {
    my ( $degree, $minute ) = @_;
    $location{'yy0'} = '-1';
    $location{'yy1'} = $degree;
    $location{'yy2'} = $minute;
    return;

    }

    sub get_long_E {
    my ( $degree, $minute ) = @_;
    $location{'zz0'} = '1';
    $location{'xx0'} = '1';
    $location{'xx1'} = $degree;
    $location{'xx2'} = $minute;
    return;
    }

    sub get_long_W {
    my ( $degree, $minute ) = @_;
    $location{'zz0'} = '-1';
    $location{'xx0'} = '-1';
    $location{'xx1'} = $degree;
    $location{'xx2'} = $minute;
    return;

    }

    sub get_sunrise {

    my $ua = LWP::UserAgent->new;
    $ua->agent('Mozilla/4.73');
    my $req =
    HTTP::Request->new(
    GET => 'http://aa.usno.navy.mil/data/docs/RS_OneDay.html' );
    my $res = $ua->request($req);

    my @form = HTML::Form->parse( $res->content, $res->base() );

    #xxy is year
    #xxm is month
    #xxd is day
    $form[1]->value( 'xxy', "2007" );
    $form[1]->value( 'xxm', "2" );
    $form[1]->value( 'xxd', "20" );

    #xx0 is -1 or 1 long 1 for east -1 for west
    #xx1 is long in degrees
    #xx2 is long in minites

    $form[1]->value( 'xx0', $location{'xx0'} );
    $form[1]->value( 'xx1', $location{xx1} );
    $form[1]->value( 'xx2', $location{'xx2'} );

    #yy0 is -1 for west 1 for east latitude
    #yy1 is degrees
    #yy2 is minites

    $form[1]->value( 'yy0', $location{'yy0'} );
    $form[1]->value( 'yy1', $location{'yy1'} );
    $form[1]->value( 'yy2', $location{'yy2'} );

    #zz1 is offset from GMT
    #zz0 is -1 west of GMT 1 east

    $form[1]->value( 'zz1', $location{'zz1'} );
    $form[1]->value( 'zz0', $location{'zz0'} );

    $req = $form[1]->make_request;
    $req = $ua->request($req);
    $req->content =~ /\s+Sunrise\s+(\d+:\d+)\s+/;

    #print $req->content;
    my $sunrise = $1;
    $req->content =~ /\s+Sunset\s+(\d+:\d+)\s+/;
    my $sunset = $1;
    return ( $sunrise, $sunset );
    }

    __DATA__
    Aberdeen, Scotland 57 9 N 2 9 W
    Adelaide, Australia 34 55 S 138 36 E
    Algiers, Algeria 36 50 N 3 0 E
    Amsterdam, Netherlands 52 22 N 4 53 E
    Ankara, Turkey 39 55 N 32 55 E
    Asuncion, Paraguay 25 15 S 57 40 W
     
    Mothra, Apr 12, 2007
    #1
    1. Advertising

  2. Mothra

    Uri Guttman Guest

    >>>>> "M" == Mothra <> writes:

    M> I have a perl script (see below) that uses dispatch tables to set
    M> values, however subs get_lat_N and get_lat_S are almost identical
    M> :-( this tells me thatt I am doing something wrong. How can I
    M> modify my dispatch table structure to avoid duplicate code?

    well, the simple answer is get rid of the dispatch tables as you don't
    need them. they are not the best way to do this as you can simplify the
    whole script by folding get_lat_N and get_lat_S into get_lat (same for
    the EW subs). the dispatch table you have effectively just selects 1 or
    -1 based on the compass value. you can use a hash for that or do a
    simple test inside the subs.


    M> my %dispatch_table_lat = (
    M> 'N' => \&get_lat_N,
    M> 'S' => \&get_lat_S
    M> );
    M> my %dispatch_table_lon = (
    M> 'E' => \&get_long_E,
    M> 'W' => \&get_long_W
    M> );

    M> $dispatch_table_lat{$5}->( $3, $4 );
    M> $dispatch_table_lon{$8}->( $6, $7 );


    $5 and $8 have NW and EW so why not just pass them as additional args
    like this:


    get_lat_NS( $3, $4, $5 );
    get_lon_EW( $6, $7, $8 ) ;


    M> sub get_lat_N {
    M> my ( $degree, $minute ) = @_;
    M> $location{'yy0'} = '1';
    M> $location{'yy1'} = $degree;
    M> $location{'yy2'} = $minute;
    M> return;
    M> }

    M> sub get_lat_S {
    M> my ( $degree, $minute ) = @_;
    M> $location{'yy0'} = '-1';
    M> $location{'yy1'} = $degree;
    M> $location{'yy2'} = $minute;
    M> return;

    M> }

    merge those two into this:

    sub get_lat_NS {
    my ( $degree, $minute, $compass ) = @_;

    $location{'yy0'} = ( $compass eq 'N' ? '1' : '-1' ;
    $location{'yy1'} = $degree;
    $location{'yy2'} = $minute;
    return;
    }

    do the same for the longitude subs.

    so you lose 2 subs, 2 useless dispatch tables and have cleaner code.

    a dispatch table is more useful when you have more than 2 subs to call
    based on a key. and they should be doing different things. if they are
    very similar as yours are, it is better to put the conditional logic in
    the subs themselves as then you only have to change 1 sub when your
    requirements change.

    and why is the 1/-1 value in quotes? do you need it as a string? it
    seems to be a form value so maybe that is ok.


    --
    Uri Guttman ------ -------- http://www.stemsystems.com
    --Perl Consulting, Stem Development, Systems Architecture, Design and Coding-
    Search or Offer Perl Jobs ---------------------------- http://jobs.perl.org
     
    Uri Guttman, Apr 12, 2007
    #2
    1. Advertising

  3. Mothra

    Mothra Guest

    Hi Uri,

    "Uri Guttman" <> wrote in message
    news:...
    >>>>>> "M" == Mothra <> writes:

    >

    (snipped)
    >
    > well, the simple answer is get rid of the dispatch tables as you don't
    > need them. they are not the best way to do this as you can simplify the
    > whole script by folding get_lat_N and get_lat_S into get_lat (same for
    > the EW subs). the dispatch table you have effectively just selects 1 or
    > -1 based on the compass value. you can use a hash for that or do a
    > simple test inside the subs.
    >
    >

    (More snippage)
    Ok I understand

    >
    > $5 and $8 have NW and EW so why not just pass them as additional args
    > like this:
    >
    >
    > get_lat_NS( $3, $4, $5 );
    > get_lon_EW( $6, $7, $8 ) ;


    Ok so far

    >
    >

    (more snippage)
    >
    > merge those two into this:
    >
    > sub get_lat_NS {
    > my ( $degree, $minute, $compass ) = @_;
    >
    > $location{'yy0'} = ( $compass eq 'N' ? '1' : '-1' ;
    > $location{'yy1'} = $degree;
    > $location{'yy2'} = $minute;
    > return;
    > }


    works perfectly :)


    >
    > do the same for the longitude subs.


    I see, like this

    sub get_long_EW {
    my ( $degree, $minute, $compass ) = @_;
    $location{'zz0'} = ($compass eq 'E' ? '1' : '-1');
    $location{'xx0'} = $location{'zz0'};
    $location{'xx1'} = $degree;
    $location{'xx2'} = $minute;
    return;
    }

    >
    > so you lose 2 subs, 2 useless dispatch tables and have cleaner code.
    >
    > a dispatch table is more useful when you have more than 2 subs to call
    > based on a key. and they should be doing different things. if they are
    > very similar as yours are, it is better to put the conditional logic in
    > the subs themselves as then you only have to change 1 sub when your
    > requirements change.


    Thanks for the clarification :) makes perfect sence


    >
    > and why is the 1/-1 value in quotes? do you need it as a string? it
    > seems to be a form value so maybe that is ok.

    Yes, I do need it as a string. it is a form value.

    As always your advise is right on!

    Thanks

    Mothra
     
    Mothra, Apr 12, 2007
    #3
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. The Stevemeister
    Replies:
    4
    Views:
    351
    Mike Smith
    Nov 15, 2004
  2. Michele Simionato

    dispatch tables in Python 2.4

    Michele Simionato, Aug 7, 2004, in forum: Python
    Replies:
    1
    Views:
    504
    Paul Moore
    Aug 7, 2004
  3. Replies:
    4
    Views:
    5,657
  4. jiccab
    Replies:
    2
    Views:
    333
    jiccab
    Aug 11, 2006
  5. mmccaws2
    Replies:
    2
    Views:
    96
    mmccaws2
    Apr 9, 2008
Loading...

Share This Page