3
\$\begingroup\$

In an effort to keep all of the Google Maps Javascript API in it's own little world, I have created a googleMaps object that will contain all of the functions that directly make google maps webservice calls. This object will be imported once on every page. For now, it only has one function.

if (googleMaps === undefined || googleMaps === null)
{
  var googleMaps = {

    /**
    * Obtains the latitude and longitude of a location and then processes it with a callback function
    * @param {String} addressStr : comma separated string containing all pertinent address information
    * @param {Function} callback : handle to the function to call once the data lat/lng data has been retrieved successfully
    */
    getLatLngFromAddressString : function(addressStr, callback)
    {
      var geocoder = new google.maps.Geocoder();
      geocoder.geocode({'address': addressStr}, function(results, status) {
        if (status === google.maps.GeocoderStatus.OK)
        {
          var latitude = results[0].geometry.location.lat();
          var longitude = results[0].geometry.location.lng();
          callback({lat: latitude, lng: longitude});
        }
        else {
          // I have a custom error logging framework set up, but for simplicity i'll just console.log in code review snippet
          console.log("error");
        }
      });
    }
  }
}

In a separate file, I will call this function from verifyLocation(), shown below:

var showLocationCallback = function(loc)
{
  var latitude = loc.lat;
  var longitude = loc.lng;

  //other not important, totally unrelated processing stuff here
}

/**
 * makes a google maps geocode request for the provided address string
 */
function verifyLocation(addressStr)
{
  document.getElementById('verify-location').disabled = true;

  var location = googleMaps.getLatLngFromAddressString(addressStr, showLocationCallback);

}

I'm looking to get this bit of code reviewed. I'm looking for any and all improvements, including:

  • making the code more efficient

  • rethinking the logic

  • completely restructuring the way I'm approaching this

  • best coding practices / naming conventions

  • -
\$\endgroup\$
  • \$\begingroup\$ May I assume you don't use babel or traceur at all? \$\endgroup\$ – Randy May 24 '16 at 9:44
  • \$\begingroup\$ I don't even know what those are, so no I don't use them ;) \$\endgroup\$ – jros May 24 '16 at 14:12
1
\$\begingroup\$
  1. You could, but don't have to, check your googleMaps variable to be falsy, instead of undefined or null.
  2. To make this code more readable, you could seperate the function from your object. That way, if you have more functions, you have an overview of your object's implementation.

1.

old

if (googleMaps === undefined || googleMaps === null)

new

if(!googleMaps)


2.

old

var googleMaps = {
  getLatLngFromAddressString : function(addressStr, callback){
  //...    
  }
}

new

var getLatLngFromAddressString = function(addressStr, callback){
//...    
}

var googleMaps = {
  getLatLngFromAddressString : getLatLngFromAddressString
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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