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

Google Maps API latitude/longitude pair to Google Fusion Tables query

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

Problem

I'm trying to learn JavaScript, and as a practice tool I recently wrote a simple web app that uses the Google Maps API Geocoder to get the latitude and longitude of a location (user input), manipulate them, then query those against a Google Fusion Table listing the world's climate zone codes according to the Köppen-Geiger climate classification. Basically people can type in a location and find out what climate zone it is, and see a map with the zones overlaid if they want it (might be of interest to geography buffs).

All of the logic is in a JavaScript file copied below. It was my goal to write as much of it without jQuery as possible, so I only use jQuery to handle the Fusion Tables request and to add/remove classes.

I've run it through JShint, and fixed most of the things mentioned there. I had the hardest time handling the list of alternative locations that Google returns, and did it with some funky global variables, a function within a for loop, and some other certainly coding practices. Those are changeMapLocation() and buildAltLocationList().

Everything works like I want it to, but this is a learning project I'd appreciate any suggestions for refactoring, design factors, maintainability, etc.

```
/* Google Address to coordinates code from:
http://krasimirtsonev.com/blog/article/GoogleMaps-JS-API-address-to-coordinates-transformation-text-to-LatLng
*/
(function () { // Begin scoping function
"use strict";
var map = null;
var returnedLocation;
var altLocationsList;
var isAlt = false;
var markersArray = [];

/Initializing the map and input elements/

window.onload = function () {
// initialize the map and set options
var mapHolder = document.getElementById("map-holder");
map = new google.maps.Map(
mapHolder,
{
zoom: 3,
mapTypeId: google.maps.MapTypeId.ROADMAP,
zoomControl: true,
zoomControlOptions: {

Solution

From a once over:

  • I like 'use strict';



  • You have a few magic constants



  • zoom: 3,



  • new google.maps.LatLng(20.2, 0.1)



  • Consider addEventListener or jQueryevent handling instead of assigning listener directly like here:



document.getElementById("search").onclick

  • In my mind, if you use jQuery at all you should it for looking up elements.


document.getElementById("search") <> $("#search")

  • Double newlines to separate code might be over the top, I would stick to single newlines



-
I would write the following with a ternary:

var arrIndex;
if (isAlt === false ) {
    arrIndex = 1;
}
else {
    arrIndex = 0;
}


could be

var arrIndex = isAlt?0:1;


  • Your rounder calls determineClimateZone, that should not be the job of rounder



  • I would have called rounder roundCoordinates



  • I would play around with having 1 function to set locationName, zoneDescription instead of 2, you are now mixing UI and model in describeClimatezone



  • From a much higher level, if ever your lookup table changes from rounding to .25 and .75 (which is such a weird choice, so it seems likely to happen), then your code is broken. I would spend some time to analyze the fusion table and find the correct entry independent of rounding choices.

Code Snippets

var arrIndex;
if (isAlt === false ) {
    arrIndex = 1;
}
else {
    arrIndex = 0;
}
var arrIndex = isAlt?0:1;

Context

StackExchange Code Review Q#71978, answer score: 3

Revisions (0)

No revisions yet.