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

Adding markers to a Google Maps map from a source

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mapgooglemarkerssourcemapsaddingfrom

Problem

I decided to make a simple project to learn the Google Maps API. I'd appreciate a review of the code below. I'd like to know what I did right and wrong.

The code pulls data from one source and adds markers on a map via the Google Maps API. A working sample is located here: www.seedtip.com

```
$(function() {
//var geocoder = new google.maps.Geocoder();

var marketId = []; //returned from the API
var allLatlng = []; //returned from the API
var allMarkers = []; //returned from the API
var marketName = []; //returned from the API
var infowindow = null;
var pos;
var userCords;
var tempMarkerHolder = [];

//Start geolocation

if (navigator.geolocation) {

function error(err) {
console.warn('ERROR(' + err.code + '): ' + err.message);
};

function success(pos){
userCords = pos.coords;
}

// Get the user's current position
navigator.geolocation.getCurrentPosition(success, error);
//console.log(pos.latitude + " " + pos.longitude);
} else {
alert('Geolocation is not supported in your browser');
}

//End Geo location

//map options
var mapOptions = {
zoom: 5,
center: new google.maps.LatLng(37.09024, -100.712891),
panControl: false,
panControlOptions: {
position: google.maps.ControlPosition.BOTTOM_LEFT
},
zoomControl: true,
zoomControlOptions: {
style: google.maps.ZoomControlStyle.LARGE,
position: google.maps.ControlPosition.RIGHT_CENTER
},
scaleControl: false,

};

//Adding infowindow option
infowindow = new google.maps.InfoWindow({
content: "holding..."
});

//Fire up Google maps and place inside the map-canvas div
map = new google.maps.Map(document.getElementById('map-canvas'), mapOptions);

//grab form data
$('#chooseZip').submit(function() { // bind function to submit event of form

//define and set variables
var userZip = $("#textZip").val();

Solution

This is pretty nice so far. The code is easy to read, clear, straightforward.

The single biggest problem is probably this:

var mapOptions = {
    // ... a bunch of fields here
    scaleControl: false,  // <- remove that comma!
};


Many nice browsers let you get away with it, but that comma at the end is incorrect syntax. In some browsers (and non-browsers like IE) this can crash the script with cryptic error messages (happened to me, wasn't fun).

When iterating over the fields of a JavaScript object, you have to check if the field really belongs to that object, like this:

for (var key in data) {
    if (!data.hasOwnProperty(key)) {
        continue;
    }
    // ... 
}


For more details, see this post.

Declare variables in the smallest possible scope where they are used.
For example the marketId array is not used outside the AJAX call,
so it would have been better to declare there, and right before needed.
Same goes for allLatlng, allMarkers, and possible others too I didn't verify.

This URL appears multiple times with minor variations:

accessURL = "http://search.ams.usda.gov/farmersmarkets/v1/data.svc/zipSearch?zip=" + userZip;


It would be better to define a baseURL to avoid duplicated string literals:

var baseURL = "http://search.ams.usda.gov/farmersmarkets/v1/data.svc";


It may seem a minor thing,
but the indentation is not consistent,
for example it's inexplicably reduced by one level at the start of var mapOptions = ...,
then after a couple of lines it goes back to normal.
It would improve readability to be consistent with that.

Unused local variable:

var pos;


Unnecessary semicolon at the end of function declaration:

function error(err) {
    console.warn('ERROR(' + err.code + '): ' + err.message);
};


There's one more, at the end of the for (var key in data) { ... }; loop.

Code Snippets

var mapOptions = {
    // ... a bunch of fields here
    scaleControl: false,  // <- remove that comma!
};
for (var key in data) {
    if (!data.hasOwnProperty(key)) {
        continue;
    }
    // ... 
}
accessURL = "http://search.ams.usda.gov/farmersmarkets/v1/data.svc/zipSearch?zip=" + userZip;
var baseURL = "http://search.ams.usda.gov/farmersmarkets/v1/data.svc";
function error(err) {
    console.warn('ERROR(' + err.code + '): ' + err.message);
};

Context

StackExchange Code Review Q#67369, answer score: 3

Revisions (0)

No revisions yet.