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

Google Maps JavaScript API v3: Sorting Markers with Check Boxes

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

Problem

Recently I built a map with custom markers (pins) and a sorting function to display each by type (Member, Supplier, Prospect, New Member).

The HTML (check boxes wired up with sort() function):


          Suppliers


The JavaScript:

```
var markersArray = [];

// data schema ['Company Name',"Address",Latitude,Longitude,"Type"]
var data = [
['Company A',"Address",Latitude,Longitude,"Member"],
['Company B',"Address",Latitude,Longitude,"Supplier"],
['Company C',"Address",Latitude,Longitude,"Prospect"],
];

function initialize(data) {

var mapOptions = {
zoom: 3,
center: new google.maps.LatLng(40, -90)
}
var map = new google.maps.Map(document.getElementById('map-canvas'), mapOptions);

// Create the legend with content
var legend = document.createElement('div');
legend.id = 'legend';

var content = [];
content.push('Legend');
content.push(' Members');
content.push(' Suppliers');
content.push(' New Members');
content.push(' Prospects');

legend.innerHTML = content.join('');
legend.index = 1;

// call the legend
map.controls[google.maps.ControlPosition.RIGHT_BOTTOM].push(legend);

setMarkers(map, data);

}

function setMarkers(map, locations) {

if (locations == undefined) {
locations = data;
return initialize(locations);
}

for (var i = locations.length - 1; i >= 0; i--) {
var company = locations[i],
pinColor = '';

var myLatLng = new google.maps.LatLng(company[2], company[3]);
var marker = new google.maps.Marker({
position: myLatLng,
map: map,
icon: image,
shape: shape,
title: company[0],
type: company[4]
});

markersArray.push(marker);

if ( marker.type === 'Member' ) { pinColor = 'blue' }
else if ( marker.type === 'New') { pinColor = 'purple' }
else if ( marker.type === 'Supplier') { pinColor = 'red' }
else { pinColor = 'green' }

var

Solution

Let's be frank - this code is a mess.

-
A bunch of global variables and functions. That's never good. You should at least wrap these inside a container function. Or better - turn it into an object.

-
In the middle of the function definitions there's suddenly call to google.maps.event.addDomListener(). Don't mix the definitions and code that does something. Bring all the initialization into one area.

-
You knew already that the backwards for-loop is troublesome. You should have fixed it before submitting. Don't submit code that you know is poor.

-
But when I look at the end of this for-loop... WTF? You are creating an image object and then you just throw it away. Is this code unfinished?

-
For some reason the getCheckedBoxes function is defined inside sort function. Why?

-
Inside clearOverlays you reset the markersArray by doing markersArray.length = 0. That's not a proper way to reset an array. Instead just assign an empty array to replace the old one: markersArray = [].

-
resetPins is just an alias to clearOverlays. Besides it's not used. Throw away the dead code.

-
The sort function doesn't seem to be doing any sorting. Don't call it sort if it just switches groups of markers on and off.

-
Too many other issues to mention...

Most importantly, try to care about your code. If you put your code out for others to see (by posting it here), try to first make it as good as you possibly can. Remove dead code. Fix the problems that you know about. Post a complete example, not half-finished one. Structure it so that it's as easy to read and understand as possible. Run it through JSHint and fix all the warnings, so you that people will not need to point out the mistakes that a stupid machine could have told you about.

Context

StackExchange Code Review Q#46371, answer score: 4

Revisions (0)

No revisions yet.