MVC Catalyst Architecture - City, State Zip locator

[ChristinaH]

I'm fairly new Perl so any help on working this out is awesome.

The application I'm working on is based on catalyst MVC architecture. My Postgresql table looks like this:

CREATE TABLE zips (
    zip_code CHAR(5) NOT NULL,
    zip_type CHAR(1),
    city_name VARCHAR(64),
    city_type CHAR(1),
    state_name VARCHAR(64),
    state_abbr CHAR(2) NOT NULL,
    area_code CHAR(3),
    latitude NUMERIC(9,6) NOT NULL,
    longitude NUMERIC(9,6) NOT NULL
);

CREATE INDEX zip_codes ON zips (zip_code);
CREATE INDEX state_abbrs ON zips (state_abbr);
CREATE INDEX city_names ON zips USING btree (city_name); 

CREATE TABLE distance_cache (
    orig_zip CHAR(5) NOT NULL,
    dest_zip CHAR(5) NOT NULL,
    miles SMALLINT
);

My search by zipcode is working great and turning out the results I need. Searching by city and state however, is not working. This started not working when I had to create a backup of zips called zips_backup because I corrupted the zips table. I changed a perl module to pull from zips_backup . As far as I can tell, this one module is the only thing that was changed but some insight would be great.

I can supply any files that one would need. Thank you in advance - I look forward to learning whats going on with this.
Here is the module I'm referring to:

package WeilMclainDB::Zips;

use base qw/DBIx::Class/;

__PACKAGE__->load_components(qw/PK::Auto Core/);

__PACKAGE__->table('zips');

__PACKAGE__->add_columns(qw/zip_code zip_type city_name city_type stat
+e_name state_abbr area_code latitude longitude/);

__PACKAGE__->resultset_class('WeilMclainDB::ZipsResultSet');

1;

package WeilMclainDB::ZipsResultSet;
use base 'DBIx::Class::ResultSet';
use Geo::Distance;

sub get_zip {
    my ($self, $city, $state) = @_;
    #return undef unless ($city && $state =~ /^(\d*)$/);

    if (my $zip = $self->single({city_name => uc($city),state_abbr=>uc
+($state)},{columns => [qw/zip_code/]})) {
        return $zip->get_column('zip_code');
    }

    return undef;
    return $self->single({city_name=>uc($city),state_abbr=>uc($state)}
+,{columns=>[qw/zip_code/]})->get_column('zip_code');
}

sub get_closest {
    my ($self, $zip_code, $miles) = @_;

    # First get the latitude and longititude for the given zip code
    if (my $zip = $self->single({zip_code => $zip_code})) {
        my $geo = Geo::Distance->new;
        my $locations = $geo->closest(
            dbh => $self->result_source->storage->dbh,
            table => 'zips',
            lon => $zip->longitude,
            lat => $zip->latitude,
            unit => 'mile',
            distance => $miles,
            lon_field => 'longitude',
            lat_field => 'latitude',
            'sort' => 1,
            fields => [qw/zip_code/],
        );
    
        my @zips;
        foreach my $result (@{$locations}) {
            push (@zips, $result->{zip_code});
        }
        return \@zips;
    }
    else {
        return undef;
    }
}

1;

[Your Mother]

When something goes from working to not working and you think you didn't change anything significant, you usually just made one dumb little mistake. If you want debugging to be easy, put lots of feedback in your code to report when it's used incorrectly (and write tests for it). Try this for starters-

use Carp;

sub get_zip {
    my $self = shift;
    my $city = uc(+shift) || croak "No city given";
    my $state = uc(+shift) || croak "No state given";

    my $zip = $self->single({ city_name => $city,
                              state_abbr => $state },
                            { columns => [qw/zip_code/] }
                           );
    if ( $zip )
    {
        return $zip->zip_code;
    }
    else
    {
        carp "No ZIP found for city_name:$city and state_abbr:$state";
    }
    return undef;
}

I'm a fan of the die early, die often edict (hence the croaks) but you can tone it down or adapt. The main idea is just to never assume that your code will be called as designed and account for that. See also Params::Validate.

[Anonymous Monk]

I'm a regular commenter when using PHP and if I knew Perl at all I would comment it out as well. Thats one thing that has been so difficult with this script and the models with it. Thanks for your help!

[Your Mother]

I'd say, general rule, avoid comments in Perl in favor of Pod. Comments have their place (and opinions differ widely) but I personally only like to see them when they illuminate some data or business logic oddity and as you'll read here, they can lie or be out of sync just as often as they help. Well chosen variable and sub names are better than comments most of the time.

Here are examples of both. See perlpod for more.

# Perl only has one real comment style, this is it.
# Leading pound/number sign.

=head2 get_zip

L</get_zip> takes blah, blah, and returns blah, blah, blah.

Pod has more syntax and is readily convertible to a number of formats 
+from RTF to WikiText and HTML and plenty in between.

=cut

sub get_zip {
    my $self = shift;
# ...

[jethro]

"is not working". This is like telling the doctor 'I'm sick' ;-). You could give a bit more detail about how it is not working.

Some other ideas:

You might test the database by giving the SQL commands probably used in a search directly to postgres.

You might try to call your program from the command line using the build-in perl debugger. Do single step until you arrive at the places in the model where SQL commands are executed. Check these SQL statements. See http://search.cpan.org/~zarquon/Catalyst-Manual-5.7013/lib/Catalyst/Manual/Tutorial/Debugging.pod

Check the documentation of Catalyst for build-in debug logging. See http://www.perl.com/lpt/a/930

[Anonymous Monk]

Its not working in that the search by city and state will not turn out results. Searching by zip code will. I've tried out the debugging last Friday but I kept getting debugging errors on my connection files since I was working locally. But I will look into the catalyst debugging.

[friedo]

I changed a perl module to pull from zips_backup

Are you sure about that? The DBIC module you pasted has

__PACKAGE__->table('zips');

[ChristinaH]

Yes.. the old one that I did referred to "zips_backup". As of today I have restored the original zips table. so all is good with that now.

[pileofrogs]

From what little I understand about these fancy MVC thingies, changing the name of something can have far-reaching consequences. EG the table name and the form name have to be the same or lightning comes out of your telephone and eats your hat...

[ChristinaH]

Ohhh.. believe me, I've been without a hat since I broke the fancy thing. The table name and the only file it points to (zips.pm) match. So I'm really at a lost. Here is the controller if you're interested.
Read more... (2 kB)

[dotcomguy]

This is code that I originally developed for WeilMclain. If the problem is city/state searches then you need to look at get_zip() because it has to search the zips table to find a zip code for the given city and state. If that doesn't find a zip code for a given city/state it won't work. I also have the original file as well if you need it. Feel free to contact me at [e-mail] for more assistance

[Anonymous Monk]

Thats what I was understanding when looking at the code. To me and a few people who have looked at it - its fine and should be working. Thats why were stumped. It seems there is something fouling it up that isn't obviously apparent. Perhaps something outside the WeilMclain and WeilMclainDB folders. Thank you much for your help with this. I emailed you a bit ago.

[Anonymous Monk]

Problem solved! Here is what changed in the zips.pm

sub get_zip {
    my ($self, $city, $state) = @_;
$cityy=   s/\b(\w)(\w*)/uc($1).lc($2)/ge for $city;

#print "$city";
#    return undef unless ($city && $state =~ /^(\d*)$/);
    if (my $zip = $self->single({city_name => $city,state_abbr=>uc($st
+ate)},{columns => [qw/zip_code/]})) {
    return $zip->get_column('zip_code');
}
    return undef;
    return $self->single({city_name=>$city,state_abbr=>uc($state)},{co
+lumns=>[qw/zip_code/]})->get_column('zip_code');
}

Thank you all for your help!