HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

JavaScript geocoder

Submitted by: @import:stackexchange-codereview··
0
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 #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 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.