patternjavascriptMinor
JavaScript geocoder
Viewed 0 times
javascriptgeocoderstackoverflow
Problem
I'm pretty excited to have coded my first JavaScript file which now works, and I've spent my evening trying to improve it.
It takes a location in the
I've worked hard on this but I am really interested to see if any JavaScript gurus can spot any mistakes, because I love beautiful code.
jsFiddle
```
var coded = false;
geocode();
$.cookie("country", "uk");
// GEOCODE FUNCTION
function geocode() {
var input = $('#loc')[0];
var options = {types: ['geocode']};
var country_code = $.cookie('country');
if (country_code) {
options.componentRestrictions = {
'country': country_code
};
}
var autocomplete = new google.maps.places.Autocomplete(input, options);
google.maps.event.addListener(autocomplete, 'place_changed', function() {
processLocation();
});
$('#searchform').on('submit', function(e) {
if (coded === false) {
processLocation();
}
return true;
});
$("#loc").bind("change paste keyup", function() {
coded = false;
});
}
function processLocation() {
var geocoder = new google.maps.Geocoder();
var address = $('#loc').val();
geocoder.geocode({
'address': address
},
function(results, status) {
if (status === google.maps.GeocoderStatus.OK) {
coded = true;
$('#lat').val(results[0].geometry.location.lat());
$('#lng').val(results[0].geometry.location.lng());
} else {
coded = false;
alert("Sorry - We couldn't find this location. Please try an alternative
It takes a location in the
#loc input box and geocodes it (gets coordinates from a location) when the search button is pressed, or when the autosuggestion is clicked, then submits the form.I've worked hard on this but I am really interested to see if any JavaScript gurus can spot any mistakes, because I love beautiful code.
jsFiddle
```
var coded = false;
geocode();
$.cookie("country", "uk");
// GEOCODE FUNCTION
function geocode() {
var input = $('#loc')[0];
var options = {types: ['geocode']};
var country_code = $.cookie('country');
if (country_code) {
options.componentRestrictions = {
'country': country_code
};
}
var autocomplete = new google.maps.places.Autocomplete(input, options);
google.maps.event.addListener(autocomplete, 'place_changed', function() {
processLocation();
});
$('#searchform').on('submit', function(e) {
if (coded === false) {
processLocation();
}
return true;
});
$("#loc").bind("change paste keyup", function() {
coded = false;
});
}
function processLocation() {
var geocoder = new google.maps.Geocoder();
var address = $('#loc').val();
geocoder.geocode({
'address': address
},
function(results, status) {
if (status === google.maps.GeocoderStatus.OK) {
coded = true;
$('#lat').val(results[0].geometry.location.lat());
$('#lng').val(results[0].geometry.location.lng());
} else {
coded = false;
alert("Sorry - We couldn't find this location. Please try an alternative
Solution
Looking at your code, the first thing that stands out is the
A simpler, more robust solution would be to store the last searched-for location in the
The second thing I notice is that there's some duplication going on. For instance, getting
Lastly, I'm afraid there's an outright bug. You use
Here's a rewrite
coded variable. You're checking or setting its value in many different places, which is the sort of thing you want to avoid, as it can quickly lead to errors. You're manually keeping track of the state.A simpler, more robust solution would be to store the last searched-for location in the
processLocation function, and if it's asked twice or more for that same location, give the same result as the first time. That way, the logic is all in one place.The second thing I notice is that there's some duplication going on. For instance, getting
$("#loc") several times, and in different functions. Another thing you'll want to avoid. It's the DRY principle: Don't repeat yourself. Lastly, I'm afraid there's an outright bug. You use
$.cookie to set the country to "uk", and that'll happen every time the page loads. Cookies exist to store data between pageviews; by overwriting any existing value with "uk" every time the page loads, what you're doing is no different from just writing var country = "uk"; and not using cookies at all. Moreover, unless there's a way for the user to choose another country, there's not even a reason to use a var much less a cookie. You could simply set componentRestrictions.country to "uk" directly.Here's a rewrite
$(function () { // this will be called when the page has loaded - and keep things contained
var input = $("#loc"), // find the elements once and for all
lat = $("#lat"),
lng = $("#lng"),
lastQuery = null,
autocomplete;
function processLocation(query) {
var query = $.trim(input.val()),
geocoder;
if( !query || query == lastQuery ) {
return; // stop here if query is empty or the same as last time
}
lastQuery = query; // store for next time
geocoder = new google.maps.Geocoder();
geocoder.geocode({ address: query }, function(results, status) {
if( status === google.maps.GeocoderStatus.OK ) {
lat.val(results[0].geometry.location.lat());
lng.val(results[0].geometry.location.lng());
} else {
alert("Sorry - We couldn't find this location. Please try an alternative");
}
});
}
// set up
autocomplete = new google.maps.places.Autocomplete(input[0], {
types: ["geocode"],
componentRestrictions: {
country: "uk"
}
});
google.maps.event.addListener(autocomplete, 'place_changed', processLocation);
$('#searchform').on('submit', function (event) {
processLocation();
event.preventDefault();
});
});Code Snippets
$(function () { // this will be called when the page has loaded - and keep things contained
var input = $("#loc"), // find the elements once and for all
lat = $("#lat"),
lng = $("#lng"),
lastQuery = null,
autocomplete;
function processLocation(query) {
var query = $.trim(input.val()),
geocoder;
if( !query || query == lastQuery ) {
return; // stop here if query is empty or the same as last time
}
lastQuery = query; // store for next time
geocoder = new google.maps.Geocoder();
geocoder.geocode({ address: query }, function(results, status) {
if( status === google.maps.GeocoderStatus.OK ) {
lat.val(results[0].geometry.location.lat());
lng.val(results[0].geometry.location.lng());
} else {
alert("Sorry - We couldn't find this location. Please try an alternative");
}
});
}
// set up
autocomplete = new google.maps.places.Autocomplete(input[0], {
types: ["geocode"],
componentRestrictions: {
country: "uk"
}
});
google.maps.event.addListener(autocomplete, 'place_changed', processLocation);
$('#searchform').on('submit', function (event) {
processLocation();
event.preventDefault();
});
});Context
StackExchange Code Review Q#26215, answer score: 3
Revisions (0)
No revisions yet.