views:

3784

answers:

5

This is kind of a brainteaser question, since the code works perfectly fine as-is, it just irritates my aesthetic sense ever so slightly. I'm turning to Stack Overflow because my own brain is failing me right now.

Here's a snippet of code that looks up an address using the Google Maps JS API and places a marker on a map. However, sometimes the initial lookup fails, so I want to repeat the process with a different address.

geocoder.getLatLng(item.mapstring, function(point) {
    if (!point) {
     geocoder.getLatLng(item.backup_mapstring, function(point) {
      if (!point) return;
      map.setCenter(point, 13);
      map.setZoom(7);
      map.addOverlay(new GMarker(point));
     })
     return;
    }
    map.setCenter(point, 13);
    map.setZoom(7);
    map.addOverlay(new GMarker(point));
})

(The second parameter to getLatLng is a callback function.)

Of course you can see that the three lines that center and zoom the map and add the marker are duplicated, once in the primary callback and once in the "fallback callback" (ha ha). Can you find a way to express the whole thing without any redundancy? You earn bonus points, and my adulation, if your solution works for an arbitrary number of backup map strings.

+1  A: 

How about this?

function place_point(mapstrings,idx)
{
    if(idx>=mapstrings.length) return;
    geocoder.getLatLng(mapstrings[idx],
                       function(point)
                       {
                           if(!point)
                           {
                               place_point(mapstrings,idx+1);
                               return;
                           }
                           map.setCenter(point, 13);
                           map.setZoom(7);
                           map.addOverlay(new GMarker(point));
                       });
}

As many backup strings as you want. Just call it with a 0 as the second argument the first time.

Jay Kominek
+1  A: 

Yep, factor it out into a function :)

geocoder.getLatLng(item.mapstring, function(point) {
    if (!point) {
        geocoder.getLatLng(item.backup_mapstring, function(point) {
                if (point) {
                    setPoint(point);
                }
        })
        return;
    }

    function setPoint(point) {
        map.setCenter(point, 13);
        map.setZoom(7);
        map.addOverlay(new GMarker(point));
    }

    setPoint(point);
});
Ricky
+3  A: 

There is an exceedingly nice method for performing recursion in language constructs that don't explicitly support recursion called a fixed point combinator. The most well known is the Y-Combinator.

Here is the Y combinator for a function of one parameter in Javascipt:

function Y(le, a) {
    return function (f) {
        return f(f);
    }(function (f) {
        return le(function (x) {
            return f(f)(x);
        }, a);
    });
}

This looks a little scary but you only have to write that once. Using it is actually pretty simple. Basically, you take your original lambda of one parameter, and you turn it into a new function of two parameters - the first parameter is now the actual lambda expression that you can do the recursive call on, the second parameter is the original first parameter (point) that you want to use.

This is how you might use it in your example. Note that I am using mapstrings as a list of strings to look up and the pop function would destructively remove an element from the head.

geocoder.getLatLng(pop(mapstrings), Y(
  function(getLatLongCallback, point)
  {
    if (!point)
    {
      if (length(mapstrings) > 0)
        geocoder.getLatLng(pop(mapstrings), getLatLongCallback);
      return;
    }

    map.setCenter(point, 13);
    map.setZoom(7);
    map.addOverlay(new GMarker(point));
  });
1800 INFORMATION
A: 

I suggest this, using a closure :

var mapstrings = ["url1","url2","url3"] ;

function getLatLng_callback(iteration, point) {
    if (!point) {
        if (mapstrings[iteration]) {
            geocoder.getLatLng(mapstrings[iteration], 
                function(point) { getLatLng_callback(iteration+1, point) ; } );
        }
        return ;
    }
    map.setCenter(point, 13);
    map.setZoom(7);
    map.addOverlay(new GMarker(point));
};

// dummy first call, let it call getLatLng
getLatLng_callback(0);
vincent
+6  A: 

The other answers are good, but here's one more option. This allows you to keep the same form you started with but uses the trick of naming your lambda function so that you can refer to it recursively:

mapstrings = ['mapstring1', 'mapstring2', 'mapstring3'];

geocoder.getLatLng(mapstrings.shift(), function lambda(point) {
   if(point) {
        // success
     map.setCenter(point, 13);
     map.setZoom(7);
     map.addOverlay(new GMarker(point));
    }
    else if(mapstrings.length > 0) {
     // Previous mapstring failed... try next mapstring
     geocoder.getLatLng(mapstrings.shift(), lambda);
    }
    else {
     // Take special action if no mapstring succeeds?
    }
})

The first time the symbol "lambda" is used, it is to introduce it as a new function literal name. The second time it is used, it is a recursive reference.

function literal naming works in Chrome, and I assume it works in most modern browsers, but I haven't tested it and I don't know about older browsers.

tway
You don't need literal naming, you can use what I used in my solution - arguments.callee refers to the function.
Jason Bunting
Literal naming is way cleaner and less chatty than your solution though
1800 INFORMATION
Naming the function instead of allowing it to refer to itself via arguments.callee is "way" cleaner? LOL - I think this is somewhat subjective, to be honest. :)
Jason Bunting
If you look at a piece of code and find yourself holding your nose, that's probably a good sign
1800 INFORMATION
That arguments.callee stuff is going to be nonobvious to anyone without a lot of Javascript experience. This is certainly more maintainable.
Jay Kominek
Named lambda expressions are something that made me think "I think all languages should have this". The arguments.callee thing made me think "What a neat hack, I could see maybe needing to use this, somewhere"
1800 INFORMATION