2
\$\begingroup\$

I hope this is the right board for this. I am a novice programmer and I am trying to learn how to write better code. I have a visualforce page with a very long Javascript script in the middle of it to display a Google Map. The code is entirely inside of a single function, but I cannot figure out a cleaner way to break it up. What are some ways that I can write code cleaner and better?

<apex:page docType="html-5.0" standardController="Property__c" extensions="ControllerJSGeo">

<head>
<script type="text/javascript" src="https://maps.googleapis.com/maps/api/js?key=XXXXXXXXXXX&callback=initMap&sensor=false"></script>


<script type="text/javascript" src="https://ajax.googleapis.com/ajax/libs/jquery/1/jquery.min.js"></script> 
<apex:includeScript value="https://c.xxx.visual.force.com/resource/1466633623000/connection"/>
<script type="text/javascript" src="/soap/ajax/24.0/apex.js"></script> 


<script type="text/javascript"> 

$(document).ready(function() {

//set map variables

var myOptions = {
    zoom: 18,
    mapTypeId: google.maps.MapTypeId.HYBRID,
    mapTypeControl: true,
    scrollwheel: false
}

var map;
var marker;
var marker2;
var propLat;
var propLong;

var geocoder = new google.maps.Geocoder();
var address = "{!Property__c.Property_Address__c}, " + "{!Property__c.City__c}, " + "{!Property__c.State__c} " + "{!Property__c.Zip_Postal_Code__c}}";

var infowindow = new google.maps.InfoWindow({
    content: "<b>{!Property__c.Name}</b><br>{!Property__c.Property_Address__c}<br>{!Property__c.City__c}, {!Property__c.State__c} {!Property__c.Zip_Postal_Code__c}"
    });       


//get the Tenant Subtype, removing any unsafe characters such as quotes
var dirtyST="{!Property__c.Tenant_Sub_Type__c}";
var subtype=cleanQuotes(dirtyST);

function cleanQuotes(unsafe) {
    return unsafe
    .replace(/'/g,"\\'");
}

    //get geocoordinates of each property            
    geocoder.geocode( { address: address}, function(results, status) {
        if (status == google.maps.GeocoderStatus.OK && results.length) {
            if (status != google.maps.GeocoderStatus.ZERO_RESULTS) {

            //create map
            map = new google.maps.Map(document.getElementById("map"), myOptions);
            map.setTilt(45);


            //center map
            map.setCenter(results[0].geometry.location);

            //create marker
            marker = new google.maps.Marker({
                position: results[0].geometry.location,
                map: map,
                title: "{!Property__c.Name}"
            });

            //add listeners
            google.maps.event.addListener(marker, 'click', function() {
                infowindow.open(map,marker);
            });

            google.maps.event.addListener(infowindow, 'closeclick', function() {
                map.setCenter(marker.getPosition()); 
            });

            //if the geocoord fields are blank, create new Salesforce object from record, and set the geocoord fields to geocoordinates from result
            propLat=results[0].geometry.location.lat();
            propLong=results[0].geometry.location.lng();

            var pLat="{!Property__c.GeoCoord__latitude__s}";
            var pLng="{!Property__c.GeoCoord__longitude__s}";


            if((!pLat||!pLng||pLat==""||pLng=="")&&(!address||address!="")){
                var newRecords = []; 
                var c = new sforce.SObject("Property__c"); 
                var pId="{!Property__c.Id}";
                c.Id=pId;
                c.GeoCoord__latitude__s=propLat;
                c.GeoCoord__longitude__s=propLong;

                //Then update the record and reload the page
                newRecords.push(c); 
                result = sforce.connection.update(newRecords); 
                reload();
            }

            else{
                console.log("geocoding failed for"+pId+"::"+address+"...Address is null or geocoords already exist");
            }

            //Get the properties returned by the Controller, and create a Marker object for each one 
            var geosArray = [];
            var geosObject={};
            var geos=[];
            var idy=0;
            <apex:repeat value="{!getgeoList}" var="m">
            geosArray[idy++]='{!m}';
            </apex:repeat>

            for (var g = 0; g < geosArray.length; ++g) {
                geosObject=JSON.parse(geosArray[g]);
                geos.push(geosObject);
            }

            for (var i = 0; i < geos.length; ++i) {
                var myLatLng = {lat: geos[i].GeoCoord__c.latitude, lng: geos[i].GeoCoord__c.longitude};
                var html="<b>Nearby {!Property__c.Tenant_Sub_Type__c}:</b><br /><br /></b><a href='/"+geos[i].Id+"'target='_blank'>"+geos[i].Name+"</a><i><br />"+geos[i].Property_Address_Full__c+"</i><br />";
                createMarker(myLatLng, html);
            }

            function createMarker(latlng, html) {
                marker2 = new google.maps.Marker({
                    position: latlng,
                    map: map,
                    icon:'http://maps.google.com/mapfiles/ms/icons/blue-dot.png'
                });

                addInfo(marker2,html);
            }

            function addInfo(marker2,html) {
                var infowindow2 = new google.maps.InfoWindow({
                    content: html                         
                });

            google.maps.event.addListener(marker2, 'click', function() {
                infowindow2.open(map, marker2);
                });
            }



//Brackets for OK results
            } 
        } 

        else {
            $('#map').css({'height' : '15px'});
            $('#map').html("Oops! {!Property__c.Name}'s billing address could not be found, please make sure the address is correct.");
            resizeIframe();
        }
    });

// resize iFrame 
    var me = window.name;
    if (me) {
        var iframes = parent.document.getElementsByName(me);
        if (iframes && iframes.length == 1) {
            height = document.body.offsetHeight;
            iframes[0].style.height = height + "px";
        }
    }


});



</script>

<style>
#map {
  font-family: Arial;
  font-size:12px;
  line-height:normal !important;
  height:800px;
  background:transparent;
}
</style>

</head>

<body>
<div id="map"></div> 
<apex:form id="distance">
    <apex:outputText >Find nearby properties within&nbsp;</apex:outputText>
    <apex:input value="{!dist}" label="X" type="number"/>
    <apex:outputText >&nbsp;miles&nbsp;&nbsp;&nbsp;&nbsp;</apex:outputText>
    <apex:commandButton value="Submit" action="{!getgeoList}"/>
</apex:form>

</body>




<br /><br />
<apex:outputText >====DEBUG====</apex:outputText><br />
<apex:outputText >Distance:</apex:outputText>
<apex:outputText >{!dist}</apex:outputText><br /><br />
<apex:outputText >{!getgeoList}</apex:outputText>

<apex:form >
    <apex:actionFunction action="{!reload}" name="reload"/>
</apex:form>


</apex:page>
\$\endgroup\$
0

1 Answer 1

Reset to default
2
\$\begingroup\$

Sure, there are many things you can clean up just in your javascript.

  • Always develop in strict mode.
  • Always use Immediately Invoked Function Executions.
  • You might not need jQuery
  • If you decide you do need jQuery, always use noConflict.
  • Always use JSLint
    • It mostly dings you for trailing space here
  • You can one-line your var declarations.

And specific to the Salesforce platform:

  • All strings used in your Javascript should be encoded with JSENCODE.
    • You don't need to reinvent the wheel with your cleanQuotes function, and it will likely delay or fail security review.
  • When passing arrays between server side and client side, use serialization and parsing instead of mixing markup into your scripts with an <apex:repeat> tag.

Using the above, your code might look more like:

(function (w, $) {
    "use strict";
    $(function () {
        var map, marker, marker2, propLat, propLong,
            myOptions = {...},
            geocoder = new google.maps.Geocoder(),
            address = '{!JSENCODE(Property__c.Property_Address__c) & JSENCODE(...) & etc.}';
        // and other properties

        geocoder.geocode(..., function () {
            // this function can be declared separately
            // instead of anonymously
        });
    });
}(window, jQuery.noConflict()));

You can also break out some of the functions you have declared into top level var declarations.

(function (w, $) {
    "use strict";
    $(function () {
        var /*other variables*/,
            createMarker = function (latlng, html) {
                // logic here
            },
            addInfo = function (marker2, html) {
                // logic here
            };
    });
}(window, jQuery.noConflict()));

You might also consider geocoding your data in a batch process, at which point you can use the out of the box <apex:map> and <apex:mapMarker> tags. They are supported for addresses you haven't geocoded yet, but you only get ten markers in that case. It looks like there is now also a tag for <apex:mapInfoWindow>

\$\endgroup\$
4
  • \$\begingroup\$ Thanks. Very helpful! So <apex:mapMarker> will work even without the geocoordinates? \$\endgroup\$
    – MRG
    Commented Oct 11, 2016 at 16:30
  • 1
    \$\begingroup\$ Yes, it will work, but you can only use ten of them in that case. \$\endgroup\$
    – Adrian Larson
    Commented Oct 11, 2016 at 16:45
  • \$\begingroup\$ I haven't used JSENCODE before. I had to change your code to: {!JSENCODE(Property__c.Property_Address__c)} etc... to get it to work. \$\endgroup\$
    – MRG
    Commented Oct 11, 2016 at 19:42
  • 1
    \$\begingroup\$ Good catch, yes you still need merge syntax. Post updated. \$\endgroup\$
    – Adrian Larson
    Commented Oct 11, 2016 at 20:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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