little code review

map.js

function onEachFeature(feature, layer) {
	/* From the perspective of reading this function, it would be easier if you think about it in a linear fashion
		 What does this function do?
		 1) get a single feature
		 2) parse some specific properties out of the feature
		 3) use those properties to generate a string of HTML
		 4) build a popup from that HTML

		 Right now its a little hard to read because you're jumping around between those steps a bit
	*/


	// Step #2 Gather properties from the feature:
	// One `var`, many variables, comma-separated
	var pdf = feature.properties.Pdf.split(','), // keep the data simple, make the app "more complex"
	    label = feature.properties.Label,
			bbox = layer.getBounds().toBBoxString(),

			/*
			This whole function can be boiled down to pretty much a single `.map` function, where you take the last result
			But aside from that, there are some things to think about in how you wrote this one:

			var preview = function() {
				this.path; << Why `this`? If it doesn't do anything for you, don't use it.
				_.each(pdf, function(item){
					this.path = 'assets/' + item + '.png';
				});
				return this.path;
			}
			*/
			preview = _.chain(pdf) // see http://underscorejs.org/#chain
				.map(function (item) { return 'assets/' + item + '.png'; }) // get an array of URLs
				.last() // take the last one
				.value(), // return the "chain's value": http://underscorejs.org/#value

			/* This is another one that boils down to an array.map function. Always use array.map if your
			goal is to take one array, process each item in it, and end up with another array

			var download_path = function(item) {
				base_uri = document.baseURI.toString(); <<< I don't think you need to make absolute URLs here
				whole_uri = base_uri.substring(0, base_uri.lastIndexOf("/") + 1);
				return whole_uri + item + '.pdf'
			}

			var links = function() {
				this.html; << the use of `this` here makes things really confusing
				var template = _.template('<ul><a href= <%= path %>> <%= name %></a></ul>'); <<< this is so simple that you don't really need templating helper
				_.each(pdf, function(item) {
					path = download_path(item);
					if (pdf.length > 1) {
						this.html += template({path: path, name: item});
					} else {
						this.html = template({path: path, name: item});
					}
				});
				return this.html;
			}
			*/

			_.each([], function () {})

			[].forEach(funtion () {})

			[].map(function () {})

			links = _.map(pdf, function (item) {
				var path = item + '.pdf';
				return '<li><a href="' + path + '">' + item + '</a></li>';
			}).join('');

			/* There's no need for a dedicated function here, because you're only going to call it once.			
			var template = function(title, preview, links, bbox) {
				return '<div class=title><h4>' + title + ' Study Area</h4></div>' +
								'<div class=content>' +
									'<div class=preview><img src=' + preview + '></div>' +
									'<div class=links><h6>Downloadable Maps:</h6>' + 
										'<div class=download>' + links + '</div>' +
										'<div id=zoom>' +
											'<button type="button" onclick="doZoom(' + bbox + ')" class="btn btn-success">Zoom to this study area</button>' +
										'</div>' +
									'</div>' +
								'</div>';
			}
			*/

	// Step #3 Turning the feature into a string of html
	// I'm starting a new `var` here just for clarity. This one is a complex bit of content
	var html = '<div class=title><h4>' + label + ' Study Area</h4></div>';
		 html += '<div class=content>';
		 html += '<div class=preview><img src=' + preview + '></div>';
		 html += '<div class=links><h6>Downloadable Maps:</h6>';
		 html += '<div class=download><ul>' + links + '</ul></div>';
		 html += '<div id=zoom>';
		 html += '<button type="button" onclick="doZoom(' + bbox + ')" class="btn btn-success">Zoom to this study area</button>';
		 html += '</div></div></div>';
	
	// Step #4 Create a popup and bind it to the layer
	var popup = L.popup({
		'maxWidth': 500,
		'minWidth': 250
	}).setContent(html);

	layer.bindPopup(popup);
}

var doZoom = function (bbox0, bbox1, bbox2, bbox3) {
	//var sw = new L.LatLng(bbox1, bbox0);
	//var ne = new L.LatLng(bbox3, bbox2);
	//var bounds = new L.LatLngBounds(sw, ne);
	// Little shortcut here
	var bounds = L.latLngBounds([[bbox1, bbox0], [bbox3, bbox2]]);
	app.map.fitBounds(bounds);
}


// I understand the effeciency here, but it takes another step to understand what's happening = harder to read
// When there are so few layers, I'd say just add them
/*
for (var key in app.layers) {
	app.layers[key].addTo(app.map);
}
*/

app.layers.baseLayer.addTo(app.map);
app.layers.fissuresLayer.addTo(app.map);
app.layers.studyAreas.addTo(app.map);
L.geocoderControl().addTo(app.map);

d3.json('data/studyareas.json', function (err, content) {

});