6
\$\begingroup\$

I've written a script with the Google Maps JavaScript API v3.

I've ran it through JSHint and everything seems to be valid.

I'm wondering if my script has room for improvement.

Any suggestions are very much welcome!

(function () {
    'use strict';

    var map,
        markers = [];

    function initMap() {
        var paris = new google.maps.LatLng(48.856614, 2.3522219000000177),
            options = {
                // required
                center: paris,
                zoom: 10,
                // disable all user interaction
                disableDefaultUI: true,
                navigationControl: false,
                mapTypeControl: false,
                scaleControl: false,
                scrollwheel: false,
                draggable: false,
                zoomControl: false,
                disableDoubleClickZoom: true
            };

        map = new google.maps.Map(document.getElementById('map'), options);

        addMarker(paris, false);

        // center map responsively
        google.maps.event.addDomListener(window, 'resize', function () {
            var center = map.getCenter();

            google.maps.event.trigger(map, 'resize');

            map.setCenter(center);
        });
    }

    function geocodeAddress() {
        var geocoder = new google.maps.Geocoder(),
            addressInp = document.getElementById('address-inp');

        geocoder.geocode({ 'address': addressInp.value }, function (results, status) {
            if (status !== google.maps.GeocoderStatus.OK) {
                alert('Geocode was not successful for the following reason: ' + status);

                return;
            }

            map.fitBounds(results[0].geometry.viewport);
            map.setCenter(results[0].geometry.location);

            deleteAllMarkers();
            addMarker(results[0].geometry.location, true);

            addressInp.value = results[0].formatted_address;
        });
    }

    function addMarker(location, animateDrop) {
        var marker = new google.maps.Marker({
            map: map,
            position: location
        });

        if (animateDrop) {
            marker.setAnimation(google.maps.Animation.DROP);
        }

        google.maps.event.addListener(marker, 'click', function () {
            if (marker.getAnimation() !== null) {
                marker.setAnimation(null);
            } else {
                marker.setAnimation(google.maps.Animation.BOUNCE);
            }
        });

        markers.push(marker);
    }

    function deleteAllMarkers() {
        for (var i = 0; i < markers.length; i++) {
            markers[i].setMap(null);
        }
    }

    google.maps.event.addDomListener(window, 'load', initMap);

    window.onload = function () {
        var addressInp = document.getElementById('address-inp'),
            searchBtn =  document.getElementById('search-btn');

        addressInp.onkeydown = function (e) {
            if (e.keyCode === 13) {
                searchBtn.click();
                addressInp.blur();
            }
        };

        searchBtn.onclick = function () {
            geocodeAddress();
        };
    };
})();
\$\endgroup\$
6
\$\begingroup\$

Your initMap function is called by the Google Maps' DOM listener. I would actually move this to the anonymous window.onload function that you have.

The reason is is more philosophical than practical. You're trying to use Google maps' api to initialize your maps objects. It seems un-intuitive that Maps should initialize maps.

If Google ever changed their API for their DOM listener, it could mess up this functionality.

The only other issue I would consider is some protection against the initMap being called twice. Just a flag at the top or something simple like that:

if (mapsInitialized) 
    return;
mapsInitialized = true;
\$\endgroup\$
  • \$\begingroup\$ Otherwise, nice code! \$\endgroup\$ – Richard Dec 16 '14 at 19:59
  • \$\begingroup\$ Thanks. I guess the same would apply for google.maps.event.addDomListener(window, 'resize', function () { //... }), putting it in window.onresize = function () { //... }; instead, next to the onload. \$\endgroup\$ – Kid Diamond Dec 16 '14 at 20:16
  • \$\begingroup\$ Hmm... Yeah, it might be a good idea. It might remove some of the google maps API overhead. \$\endgroup\$ – Richard Dec 16 '14 at 20:21
1
\$\begingroup\$

The element id 'address-inp' is used in 2 places. If one day you want to change it, you'll have to remember to change in all places. It would be better to put this in a named constant.

Some names seem unnecessarily shortened. I would spell them out, they won't be much longer but more natural:

  • addressInput instead of addressInp
  • address-input instead of address-inp
  • searchButton instead of searchBtn
  • search-button instead of search-btn
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks for your review. The DROP animation is only executed once at the beginning, and is not the same as the BOUNCE animation. The BOUNCE animation is continuous, that's why getAnimation() needs to be used to check if it's currently ongoing and put in an if-statement so that I can make the click act like a toggle. \$\endgroup\$ – Kid Diamond Dec 16 '14 at 19:38
  • \$\begingroup\$ I see your point. I dropped that from my post. \$\endgroup\$ – janos Dec 16 '14 at 19:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.