More refactorings from the trenches

A little earlier this week I came across these JavaScript functions while visiting some code in the application I’m working on:

function view_map(location) {

    window.open(http://www.google.co.uk/maps?q= + location.value);

    return false;

}

 

function view_directions(form) {

 

    var fromLocation = jQuery(form).find(".from_point").get(0);

    var toLocation = jQuery(form).find(".to_point").get(0);

 

    window.open(http://www.google.co.uk/maps?q=from:’ + fromLocation.value + ‘ to:’ + toLocation.value);

    return false;

}

 

These methods are used to open links to Google maps* pages. 

Take note – to consume either of the JavaScript functions, you have to provide specific DOM elements and the form needs to have other fields stashed away the mark-up.

So let’s see how these were being consumed:

 

<a href="#" title="Click to show map" onclick="view_map(jQuery(this).closest(‘.journey’).find(‘.to_point’).get(0));" >To</a>

 

<%–and much further down in the markup–%>

 

<input name="FromLatLng" class="hidden from_point" type="text" value="<%= Model.FromLatLng %>"/>

<input name="ToLatLng" class="hidden to_point" type="text" value="<%= Model.ToLatLng %>"/>

 

Yuck. There’s not a lot going for this code.

Particularly nasty is that code will traverse the DOM looking for particular fields. It then builds up a query string based on the field values, and once clicked, open the link in a new window.

 

Let’s try again.

In this example, the values eventually passed to the JS function,to construct the URI are actually known upfront. So instead, let’s build a small helper extension to construct our Uri from a given location. We’ll hang this helper off of the UrlHelper for convenience:

 

public static string GoogleMap(this UrlHelper urlHelper, string location)

{

    var uri = new UriBuilder("http://www.google.co.uk/maps")

                {

                    Query = string.Format("q={0}", HttpUtility.UrlEncode(location))

                };

 

    return uri.ToString();

}

 

Then, let’s consume it:

 

<a href="<%= Url.GoogleMap(Model.ToLatLng) %>"  target="_blank" title="Click to show map">To</a>

 

This is much less brittle as we no longer traverse the DOM for no reason. We can also remove the unnecessary JavaScript functions, instead using the target attribute of the anchor to provide the same functionality.

 

 

 

*Google provides means to provide maps inside your application and you should use them – using the approach shown above directs users away from your site.

Advertisements

About craigcav

Craig Cavalier works as a Software Developer for Liquid Frameworks in Houston Tx, developing field ticketing and job management solutions for industrial field service companies.

Posted on April 12, 2011, in Uncategorized. Bookmark the permalink. Leave a comment.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: