None of this code needs to create any global variables. The pattern you have here is known as namespace creation.
It often follows the pattern:
var myNamespace = (function () {
//private stuff
var public = {}; //or function () ...
//build up public object
return public;
}());
But sometimes the public object can be inlined like it can be in your case:
var geoLocateAddress = (function () {
function init() {
//...
}
return { init: init };//unnecessary to expose the other functions as public
}());
In this case, all you are doing with this namespaced global object is calling the init function. So instead of creating a global (generally considered bad taste in most languages) you can simply initialize the object:
(function () {
function init() {
//...
}
init();//unnecessary to expose the other functions as public
}());
(this pattern of having the function surrounding your code like this is commonly referred to as an IIFE)
Aside: for declaring a namespace I prefer the following form:
(function ($, window) {
'use strict';
...
window['geoLocateAddress'] = window['geoLocateAddress'] || {};
window['geoLocateAddress'] = $.extend({}, window['geoLocateAddress'], {
'init': init
};
}(jQuery, window));
Back on topic:
In my opinion, it is a bad practice to force the Javascript engine to go looking for your globals when you are not in global scope due to leaving off the window qualifier. Thus everywhere you reference console without calling it window.console (and the same goes for Navigator) or caching it in a local variable is wrong. In the event where you run this script outside of a browser that implements console you will get an exception. Navigator is slightly less of an issue because it is a reasonably safe assumption that you will not run this from a browser that doesn't supply a Navigator global namespace. For this reason I always pass the globals that I am going to use into my IIFE:
(function ($, geolocation, console) {
...
}(jQuery, Navigator.geolocation, window.console));
I would modify the rest of the code as follows; please comment if you want me to explain any of it in more detail and I'll try to get to it. Comments dictionary:
- pref: personal preference (the opinions of others may vary)
- rule: avoids possible issues with the code
- possibility: potential source for further enhancement, outside of the scope of this question.
- consideration: something I haven't decided one way or the other on yet
//pref: don't create global variables that are unnecessary
(function ($, geolocation, console) {//pref: pass in globals for munging purposes
'use strict'; //pref: 100% on JsLint
//pref: instead of defining one object variable and methods on it
//we could define the methods and export an object at the end
//this saves us from needing to do some sort of static object call syntax
var updateAddressFields = function (data) { //pref: remove unused parameters
console.log('Zip: ' + data.address.postalCode);
//pref: renaming variable provides very little benefit;
//outweighed by ability to simplify syntax by inlining to success
//function and then replacing the success function here
console.dir(data); //pref: make above a constant string and use this instead of just logging zipcode
$(function () { //rule: don't update DOM until after the ready event
$('#zip').text(data.address.postalCode);
});
//pref: further logging beyond start of function adds nothing but cruft to your log
},
loadAddressByLocation = function (position) { //pref: renamed function for readability
console.log('loadAddressByLocation:');
console.dir(position); //pref: inlined function to simplify code
var data = { //possibility: abstract this as a jQuery helper if it has uses outside of setting zipcode (outside of this question scope)
lat : position.coords.latitude,
lng : position.coords.longitude
},
url = "http://ws.geonames.org/findNearestAddressJSON"; //consideration: variable unnecessary?
$.ajax({ //consideration: ajax vs getJSON?
url: url,
dataType: 'jsonp', //consideration: specify datatype or put callback in the url?
data: data, //consideration: inline data definition?
success: updateAddressFields, //pref: extra function wrapping unnecessary
error: function (xhr, textStatus) {//perf: remove unused parameters
console.log(textStatus);//consideration: necessity of error function?
}
});
//pref: return false did nothing here; removed
};
console = console || { //rule: safe to remove shim if you know you're browser compat
log: function () {}, //otherwise console may not exist under certain browsers
dir: function () {}
};
//perf: removed and inlined "return { init: init };" to not create globals
console.log("Using your browser's GeoLocation API");//pref: inlined init and initEventHandlers for readability
if (geolocation && geolocation.getCurrentPosition) { //rule: more behavior detection, also safe to remove if browser compat
//assume correct api if this passes
geolocation.getCurrentPosition(loadAddressByLocation);
}
}(jQuery, navigator.geolocation, window.console));
//pref: passing in navigator.geolocation will allow better minification (though I wouldn't minify this version anyway)
//rule: more behavior detection, referring to console as window.console to not trigger possible error referencing global that doesn't exist
Simplified for minification, I don't really see anything "wrong" with this version:
(function ($, geolocation) { //console removed
if (geolocation) { //possibly unsafe if future browser implements geolocation but not getCurrentPosition, perhaps leave this as it was above
geolocation.getCurrentPosition(function (position) {
$.getJSON( //using this helper instead of ajax lets us not define an object and property names
"http://ws.geonames.org/findNearestAddressJSON?callback=?",
//dataType: 'jsonp', unnecessary with callback in url
{ //inlined data to remove var, IMO readability is not affected too much
lat : position.coords.latitude,
lng : position.coords.longitude
},
function (data) { //unused params dropped from success callback
$(function () {
//don't update until after the ready event
$('#zip').text(data.address.postalcode);
});
}
);
});
}
}(jQuery, navigator.geolocation));
which shrinks down to 266 chars (via uglifyjs):
(function(e,t){t&&t.getCurrentPosition(function(t){e.getJSON(
"http://ws.geonames.org/findNearestAddressJSON?callback=?",{lat:
t.coords.latitude,lng:t.coords.longitude},function(t){e(function(){
e("#zip").text(t.address.postalcode)})})})})(jQuery,navigator.geolocation)
