Can I get a second pair of eyes on this sort ...

M

MW

Found this great store locator script, tweaked it - works great -
except:

Having trouble getting it to sort by distance ($dist). I'm sure I'm
missing something stupid and thought I could get a second pair of eyes
on it to tell me what I'm missing.

#!/usr/bin/perl -w

# ------------------------------------
# Store Locator
# ------------------------------------
print "Content-Type: text/html\n\n" ;

# Retrieve zipcode from form variable.
read(STDIN, $input, $ENV{'CONTENT_LENGTH'});
foreach (split(/&/, $input))
{
($NAME, $VALUE) = split(/=/, $_);
$NAME =~ s/\+/ /g;
$NAME =~ s/%([0-9|A-F]{2})/pack(C,hex($1))/eg;
$VALUE =~ s/\+/ /g;
$VALUE =~ s/%([0-9|A-F]{2})/pack(C,hex($1))/eg;
}

$zipcode = $VALUE;

# Open DBI connection with MySQL.
use DBI;
$dbh = DBI->connect("DBI:mysql:mydb;", "myid", "mypwd")
or die "Can't connect: $_";

# Set PI and radius in miles of results to show.
$PI = 3.1415926;
$radius = 50;

# Prepare SQL.
$sth = $dbh->prepare("select city, state, latitude, longitude " .
"from zipcodes where zipcode = '$zipcode'")
or die "Can't prepare zipcode: $dbh->errstr\n";

# Execute SQL.
$rv = $sth->execute
or die "Can't execute the query: $sth->errstr\n";

($city, $state, $latitude, $longitude) = $sth->fetchrow_array;

$sth = $dbh->prepare("select store_name, store_phone, store_address1,
store_city, store_state, store_zipcode," .
" latitude, longitude from stores, zipcodes ".
"where stores.store_zipcode = zipcodes.zipcode");

$rv = $sth->execute
or die "Can't execute the query: $sth->errstr\n";

undef %store_distances;

while (($name, $s_phone, $s_address, $s_city, $s_state, $s_zipcode,
$s_lat, $s_long) = $sth->fetchrow_array)
{

# Calculate distance.
$prodsin = sin($latitude) * sin($s_lat);
$prodcos = cos($latitude) * cos($s_lat);
$deltalong = cos(abs($s_long-$longitude));
$dist = &acos($prodsin + ($prodcos * $deltalong));
$dist = $dist * (180.0/$PI) * 69.0;

$store_distances{"$name|$s_address|$s_city|$s_state|$s_zipcode|$s_phone"}
= $dist;

}

# Sort the array.
# @ordered = sort {$store_distances{$a} > $store_distances{$b}}
keys(%store_distances);
@ordered = sort keys(%store_distances);

# Print page header.
print "<html>" .
"<head>" .
"<title>" ........



# Print each record.
foreach $item (@ordered)
{
if ($store_distances{$item} <= $radius)
{



($s_name, $s_address, $s_city, $s_state, $s_zipcode, $s_phone) =
split(/\|/, $item);


print "<tr>" .
"<td>".
$s_name .
"</td>" .
"<td>" .
$s_address . "<br />" .
"$s_city, $s_state $s_zipcode <br />" .
"<a href=\"http://maps.yahoo.com/py/maps.py?Pyt=Tmap&addr=" .
$s_address .
"&csz=" .
$s_zipcode .
"&Get%A0Map=Get+Map\" target=\"_blank\" title=\"Show me a map of
this address.\">Map</a>" .
"</td>" .
"<td nowrap>" .
$s_phone .
"</td>" .
"<td align=\"right\">" .
sprintf("%d", $store_distances{$item}) .
"</td>" .
"</tr>";
}
}

# Print page footer.
print "<tr><td colspan=\"4\" align=\"center\">&nbsp;</td></tr>" .
"</body>" .
"</html>";


sub acos
{
($val) = @_;
return(atan2(sqrt(1-($val * $val)),$val));
}

sub asin
{
($val) = @_;
return(atan2($val, sqrt(1-($val * $val))));
}
 
U

Uri Guttman

M> #!/usr/bin/perl -w

no strict, no warnings.

M> # Retrieve zipcode from form variable.
M> read(STDIN, $input, $ENV{'CONTENT_LENGTH'});
M> foreach (split(/&/, $input))

broken!!
M> {
M> ($NAME, $VALUE) = split(/=/, $_);
M> $NAME =~ s/\+/ /g;
M> $NAME =~ s/%([0-9|A-F]{2})/pack(C,hex($1))/eg;
M> $VALUE =~ s/\+/ /g;
M> $VALUE =~ s/%([0-9|A-F]{2})/pack(C,hex($1))/eg;

and what if there is more than one param set? this is the worst cgi
parser i have ever seen. matt's at least (brokenly) handled multiple
values.

M> }

i refuse to look at any more code until you fix that heap of trash. use
cgi.pm


<snip of bad script kiddie code>

uri
 
S

Sam Holden

Found this great store locator script, tweaked it - works great -
except:

Having trouble getting it to sort by distance ($dist). I'm sure I'm
missing something stupid and thought I could get a second pair of eyes
on it to tell me what I'm missing.

Those two paragraphs contradict each other. Clearly if it doesn't "work great".

Ask the vendor of the script to fix it for you.

[ Snip cargo cult garbage broken perl code]
 
T

Tad McClellan

MW said:
Found this great store locator script, tweaked it - works great -


No it doesn't. You just haven't seen any of its failure modes yet.

Having trouble getting it to sort by distance ($dist). I'm sure I'm
missing something stupid


Like asking a FAQ?

on it to tell me what I'm missing.


perldoc -q sort

How do I sort a hash (optionally by value instead of key)?

#!/usr/bin/perl -w


Ask for all the help you can get:

use strict;
use warnings;

# Retrieve zipcode from form variable.
read(STDIN, $input, $ENV{'CONTENT_LENGTH'});
foreach (split(/&/, $input))


Use the CGI.pm module for parsing form values.

$store_distances{"$name|$s_address|$s_city|$s_state|$s_zipcode|$s_phone"}
= $dist;
# Sort the array.
# @ordered = sort {$store_distances{$a} > $store_distances{$b}}
^^^
^^^

That was pretty close:

@ordered = sort {$store_distances{$a} <=> $store_distances{$b}} keys...
 
M

Matt Garrish

Lori Fleetwood said:
You're such a fucking douchebag.

Holy 1980s comebacks Batman! That just cracked me up... : )

I have to say, however, that your little invective is the more pointless.
The code posted *is* crap, it's doubtful the OP has any clue how it works,
and Uri was somewhat restrained (for Uri). Rather than call people names,
you would have done more of a service to the poster by responding to his/her
question. If you had, you would have come to the same conclusion that
everyone else has.

Matt
 
L

Lori Fleetwood

Matt said:
Holy 1980s comebacks Batman! That just cracked me up... : )

Dont'cha watch'da Sopranos? Betcha Uri does, wid'this Lawn Guyland
attitude.
I have to say, however, that your little invective is the more pointless.
The code posted *is* crap, it's doubtful the OP has any clue how it works,
and Uri was somewhat restrained (for Uri). Rather than call people names,
you would have done more of a service to the poster by responding to his/her
question. If you had, you would have come to the same conclusion that
everyone else has.

I'll go further. It's ironic that the author should use the DBI but not
CGI.pm.

Another thing I might point out to the OP from a user perspective is
that I absolutely HATE store locators that require the user know the zip
code. Sometimes I'm travelling, or plan on travelling, and don't know
the zip code. These things should accept city/state, at least for the US.
 
U

Uri Guttman

LF> You're such a fucking douchebag. Just don't reply. This is why my
LF> company does google and usenet archive searches on potential
LF> consultants: as a tool in weeding out the fucking assholes. We'd
LF> rather have somebody with two thirds the talent and one third -- or
LF> less -- the attitude.

and you are a dumbshit who uses broken code and doesn't hire smart
people. nyah nyah nyah!!!

and i wouldn't work for you if you could afford me.

nyah nyah nyah!!!

and your mother has a moustache!!

nyah nyah nyah!!!

and i have more skill than you imagine, especially insulting twits like
you. nyah nyah nyah!!!!

hope you feel better soon (especially after you wake up from the crash
your broken crapola of a script brings to you).

uri
 
U

Uri Guttman

MG> Holy 1980s comebacks Batman! That just cracked me up... : )

that was funny? lori can't program her way out of the batcave with
thirty nerds in her utility belt.


MG> I have to say, however, that your little invective is the more
MG> pointless. The code posted *is* crap, it's doubtful the OP has
MG> any clue how it works, and Uri was somewhat restrained (for
MG> Uri). Rather than call people names, you would have done more of a
MG> service to the poster by responding to his/her question. If you
MG> had, you would have come to the same conclusion that everyone else
MG> has.

i am glad you agreed with the code quality. one point of karma in your
favor. but lori needs a cranial enema to clear out her attitude about
attitude. bad code deserves slamming and she doesn't get that.

ok, lori this is for you.

<sarcasm alert! sarcasm alert! sarcasm alert!>

i wish i were as nice and smart as you. i see you help people here all
the time and you fix all their bugs no matter where the code came
from. too bad i was raised in the wrong part of the world and not by
your sweet parents who bestowed their special skills on their little
baby. you handle so many perl issues with such elan and flair, it makes
me ashamed to be just a perl guru. what can i ever do to become as
useful to the perl community as you?

<end of sarcasm alert! if this had been an honest response to lori it
would have been done from beyond the grave. now back to your regular
perl flamage>

lori, too bad you don't know perl. at least moronzilla tries to leanr
new stuff (about once a year she tackles regexes and slided down the
slope back to her beloved subtr and index).

uri (having some fun tonight!)
 
M

Matt Garrish

Lori Fleetwood said:
Dont'cha watch'da Sopranos? Betcha Uri does, wid'this Lawn Guyland
attitude.

Can't say I watch much on TV but movies. The last conversation I heard with
douchebag in it went something like this:

"You're a douchebag!"
"No, *you're* a douchebag!"
"No, *YOU*'re a douchebag!"

One of the many reasons I'm glad grade school is a distant memory...
Another thing I might point out to the OP from a user perspective is
that I absolutely HATE store locators that require the user know the zip
code. Sometimes I'm travelling, or plan on travelling, and don't know
the zip code. These things should accept city/state, at least for the US.

Note to self: always be extra accomodating to Americans when building Web
sites... ; )

Matt
 
M

MW

That was pretty close:

@ordered = sort {$store_distances{$a} <=> $store_distances{$b}} keys...



Thanks, Tad - that did the trick! I knew it was going to end up being
a less-than sign somewhere.

Thanks for the tip on using CGI.pm as well. I can't believe how easy
that was to switch to and use instead.
 
T

Tad McClellan

MW said:
Thanks for the tip on using CGI.pm as well. I can't believe how easy
that was to switch to and use instead.


If you didn't know about that, then you aren't working smart yet.

If you plan to use Perl in the CGI environment, then you should have
already read the Perl FAQs about CGI:


perldoc -q CGI

How can I make my CGI script more efficient?

Where can I learn about CGI or Web programming in Perl?

What is the correct form of response from a CGI script?

My CGI script runs from the command line but not the
browser. (500 Server Error)

How can I get better error messages from a CGI program?

How do I make sure users can't enter values into a form
that cause my CGI script to do bad things?

How do I decode a CGI form?


The answer to that last one warns you off of doing exactly
what you were doing:

In short, they're bad hacks. Resist them at all costs.
 
U

Uri Guttman

M> Thanks, Martien - Tad found I was missing a "<". Now it *does* work
M> great.

M> Uh - I thought the concept behind "open source" was that if I needed a
M> script, I should go find one rather than re-invent the wheel. Well, I
M> did and it works.

there is no guarantee that open source is good code. in fact most free
perl scripts are bad code and were written by script kiddies who just
copy, paste and modify code without regard to quality, safety or
security. you picked one of those very bad scripts which did amazingly
bad cgi parsing (only one param allowed).

M> I'll agree with your suggestion that it's good practice to "use
M> strict" and that I should do that on everything that I author myself
M> from scratch. I wish everyone did that. But when I grab scripts, you
M> want me to go through them line by line and make sure everything's got
M> a "my" in front of it? I don't think I've ever grabbed any scripts
M> yet that have "use strict;".

see my point? most/all free scripts are not written strict nor warning
safe. that means they can have hard to find bugs and are very hard to
improve and modify.

M> Oh - thanks for the CGI suggestion. I can't believe how easy it was
M> to implement and how it simplified everything .

which is why we all say to use cgi.pm for basic cgi stuff like this. the
fact that none of the free scripts use it is a major red flag.

but you can get decent free cgi scripts from the nms project

http://nms-cgi.sourceforge.net

those scripts have been written and reviewed by respected members of the
perl community and are safe, secure and use modules where appropriate.

so just don't equate open source with good code. all programs, no matter
who wrote them or how much you paid for them, must be judged
individually on their own merits.

uri
 
A

Alan J. Flavell

But when I grab scripts, you want me to go through them line by
line and make sure everything's got a "my" in front of it?

If you grab that kind of script, then you must do whatever you see fit
with it... but it's probably inadvisable to discuss it here, because
the people who'll chip in with "helpful" suggestions are likely to be
those who are somewhat short of a clue, whereas the ones who could
_really_ help you will likely tell you to come back _after_ you've
started asking Perl to help you (if they tell you anything at all).
I don't think I've ever grabbed any scripts
yet that have "use strict;".

There's probably a message in there, somewhere, hmmm?

Keep in mind that many Perl scripts perform tasks that have nothing at
all to do with a web server and the CGI. And even then, they can
benefit from every help with debugging. When, however, we come to the
CGI, you are writing a script that's meant to be executed on some
_other_ authority (that of a web server), and get data from an
untrusted source (some random web visitor): the potential for mayhem
is considerable. Have you read Lincoln Stein's web security FAQ? (by
the same author as the CGI.pm module, indeed), or his book on the
topic?

As someone aptly said: "it's demeaning to ask a human to do the work
of a machine".
Oh - thanks for the CGI suggestion. I can't believe how easy it was
to implement and how it simplified everything .

In that regard I'd say you've made a good choice.

best of luck
 
U

Uri Guttman

LF> You're such a fucking douchebag. Just don't reply. This is why
LF> my company does google and usenet archive searches on potential
LF> consultants: as a tool in weeding out the fucking assholes. We'd
LF> rather have somebody with two thirds the talent and one third --
LF> or less -- the attitude.

hey lori,

where are you now? notice that the OP did change to cgi.pm as we
suggested. did your google search find that? oh, sorry, google is
looking up your rectum to find your cranium. it will be there for a
while. too bad for the google users.

i wonder who hired you with your attitude. note that your comment is
also in google's archives. does that mean you should fire yourself? or
should you just pull a nomad and sef destruct because you can't handle
the contradiction. as they say, it takes one to know one.

so ta ta for now. keep up the excellent work you do in this group. i see
you post and help people all the time with you wit and wisdom (or really
lack thereof).

have fun and don't worry about the rectal smell, your nose will get used
to it.

uri
 
L

Lori Fleetwood

Matt Garrish said:
Note to self: always be extra accomodating to Americans when building Web
sites... ; )

How about just being accomodating of users? American or not. My
comment applies equally to British and/or Canadian postal codes, the
user should not need to know the postal code in order to use the store
finder, since it could well be they are trying to find a store in an
area for which they don't know the postal code, but they are almost
certainly going to know the city and state/province/county.

So altering the script to use CGI.pm still leaves it far from optimal
from a user standpoint. The OP should know that too.
 

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,982
Messages
2,570,186
Members
46,740
Latest member
JudsonFrie

Latest Threads

Top