patternjavascriptMinor
Adding markers to a Google Maps map from a source
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();
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:
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 more details, see this post.
Declare variables in the smallest possible scope where they are used.
For example the
so it would have been better to declare there, and right before needed.
Same goes for
This URL appears multiple times with minor variations:
It would be better to define a
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
then after a couple of lines it goes back to normal.
It would improve readability to be consistent with that.
Unused local variable:
Unnecessary semicolon at the end of function declaration:
There's one more, at the end of the
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.