1
\$\begingroup\$

Even though it's simple, I've spent quite a bit of time on it, learning as I go. Is there anyway I can make this code more efficient, more crossbrowser compatible or just generally better?

jsFiddle

<!-- Geocode -->
$(document).ready(function () {
    var GeoCoded = { done: false };
    autosuggest();
    $('#myform').on('submit', function (e) {
    if (GeoCoded.done) return true;

        e.preventDefault();
        var geocoder = new google.maps.Geocoder();
        var address = document.getElementById('location').value;

        $('#myform input[type="submit"]').attr('disabled', true);

        geocoder.geocode({
            'address': address
        },

        function (results, status) {
            if (status == google.maps.GeocoderStatus.OK) {
                var latLng = results[0].geometry.location;
                $('#lat').val(results[0].geometry.location.lat());
                $('#lng').val(results[0].geometry.location.lng());
                GeoCoded.done = true;
                $('#myform').submit();
            } else {
                console.log("Geocode failed: " + status);
                //enable the submit button
                $('#myform input[type="submit"]').attr('disabled', false);
            }

        });

    });

});


function autosuggest() {
        var input = document.getElementById('location');
        var options = {
            types: [],
        };
        var autocomplete = new google.maps.places.Autocomplete(input, options);
    }
\$\endgroup\$
1
  • 1
    \$\begingroup\$ To get some quick ideas for improvements, you can try putting your code through jslint, or jshint - jshint is easier to use as it requires less set-up (for your code, you mostly just need to check the "jQuery" checkbox). \$\endgroup\$
    – Flambino
    Nov 4, 2012 at 14:55

1 Answer 1

3
\$\begingroup\$

From a once over:

  • When you wrote

    var latLng = results[0].geometry.location;
    $('#lat').val(results[0].geometry.location.lat());
    $('#lng').val(results[0].geometry.location.lng());
    

    you probably meant to write

    var latLng = results[0].geometry.location;
    $('#lat').val(latLng .lat());
    $('#lng').val(latLng .lng());
    
  • I wonder why you capture new google.maps.places.Autocomplete(input, options); into autocomplete since you do nothing with autocomplete.

  • 1 var block with comma separated variables is considered the standard, preferably on top.

  • You could capture document.getElementById('location') into a var to prevent repeated DOM lookups

  • Don't do console.log in production code

  • Creating GeoCoded as an object with 1 property is overkill, why not simply
    var geoCodingDone = false;

  • lowerCamelCasting is not applied consistently : autosuggest -> autoSuggest, autocomplete -> autoComplete

  • There is an indentation problem after return true;

With all that, I would counter suggest something like

$(document).ready(function () {

  var geoCodingDone = false,
      locationInput = document.getElementById('location'); 

  createAutoSuggest();

  $('#myform').on('submit', function (e) {
    if (geoCodingDone) 
      return true;

    var geocoder = new google.maps.Geocoder(),
        address  = locationInput.value;  

    e.preventDefault();

    $('#myform input[type="submit"]').attr('disabled', true);

    geocoder.geocode( {'address': address }, function (results, status){
        if (status == google.maps.GeocoderStatus.OK) {
            var latLng = results[0].geometry.location;
            $('#lat').val(latLng.lat());
            $('#lng').val(latLng.lng());
            geoCodingDone = true;
            $('#myform').submit();
        } else {
            $('#myform input[type="submit"]').attr('disabled', false);
        }
    });
  });
});

function createAutoSuggest(){
  var options = { types: [] };
  new google.maps.places.Autocomplete(locationInput, options);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.