I have a Google Maps Geocoder and I am looking to refactor it. The user enters address details into a form where at the bottom there is the option to show on map.
- First the map is hidden
- The user clicks
show_map
checkbox - Map-area is shown
- Map is initialized
The codeAddress()
function is used if the user edits the address and wants to find it again. The showCoords()
function passes the longitude and latitude through to the form for processing. I am looking for a best practice to refactor this code.
function initialize() {
geocoder = new google.maps.Geocoder();
codeAddress();
map = new google.maps.Map(document.getElementById('map-canvas'));
marker = new google.maps.Marker({
map: map,
position: map.getCenter(),
draggable: true
});
google.maps.event.addListener(marker, 'dragend', function() {
point = marker.getPosition();
map.panTo(point);
showCoords(point);
});
google.maps.event.addListener(map, 'dragend', function() {
point = marker.getPosition();
marker.setPosition(map.getCenter());
showCoords(point);
});
} /* END initialize */
function codeAddress(){
var address = document.getElementById('address').value;
geocoder.geocode({'address': address}, function(results, status){
if(status == google.maps.GeocoderStatus.OK){
map.setCenter(results[0].geometry.location);
map.setZoom(15);
marker.setPosition(map.getCenter());
showCoords();
}else{
alert('Geocode was not successful for the following reason: ' + status)
}
});
}
function showCoords(point){
point = marker.getPosition();
document.getElementById("lat").value = point.lat().toFixed(5);
document.getElementById("lng").value = point.lng().toFixed(5);
}
1 Answer 1
The main change I would make is to reduce the responsibility of codeAddress()
to do just the geocoding. If it successfully looks up the address, then it can pass the location to a callback.
I don't have enough context to tell whether geocoder
, map
, and marker
are global variables, or whether all of this code lies inside some other scope. Out of habit, I've converted them into local variables. You decide whether that's right for you. The point
variables in your drag handlers should definitely be made local, though!
function initialize() {
var geocoder = new google.maps.Geocoder();
var map = new google.maps.Map(document.getElementById('map-canvas'));
var marker = new google.maps.Marker({
map: map,
position: map.getCenter(),
draggable: true
});
google.maps.event.addListener(marker, 'dragend', function() {
var point = marker.getPosition();
map.panTo(point);
showCoords(point);
});
google.maps.event.addListener(map, 'dragend', function() {
var point = marker.getPosition();
marker.setPosition(map.getCenter());
showCoords(point);
});
var address = document.getElementById('address').value;
codeAddress(geocoder, address, function(location) {
map.setCenter(results[0].geometry.location);
map.setZoom(15);
marker.setPosition(map.getCenter());
showCoords();
});
}
function codeAddress(geocoder, address, callback) {
geocoder.geocode({'address': address}, function(results, status) {
if (status == google.maps.GeocoderStatus.OK) {
callback(results[0].geometry.location);
} else {
alert('Geocode was not successful for the following reason: ' + status)
}
});
}
Explore related questions
See similar questions with these tags.