1

I can't understand the execution order of the following. This part of a larger script, but the $( "#select-overlay" ) starts this going.

function findSelectedMap(mapID) {
    $.getJSON('maps.json', function (json) {
        json.forEach(function (entry) {
            if (entry.maps.id == mapID) {
                changeLayerTo = entry.maps.url;
                maxZoom = entry.maps.zoom;
            }  // end if
        }); // end json.forEach
    });    // end $.getJSON
}       // end findSelectedMap

function overlaySelector() {
    $("#select-overlay").change(function () {
        mapID = $("#select-overlay input[type='radio']:checked").val();
        findSelectedMap(mapID); // using mapID, find the url, zoom for overlayMap selected
        currentLayer = L.tileLayer(changeLayerTo).addTo(map);
        // more map stuff
    }); // end $( "#select-overlay" ).  
} // end overlaySelector function

overlaySelector is called when the page loads in a Rails app

<script>
  $(document).on("turbolinks:load", function() 
  {
     overlaySelector();
  });
</script>

I think changeLayerTo should be set on line 5 before currentLayer = L.tileLayer(changeLayerTo).addTo(map); is executed. The findSelectedMap(mapID) function is entered (I have a bunch of console.logs to track), and then execution jumps to the currentLayer = L.tileLayer(changeLayerTo).addTo(map); line (and errors, but I manually defined changeLayerTo just above so execution could continue, and then the json.forEach(function(entry) line executes and iterates over the dozen entries in maps.json and finds the correct value. But of course I need that value sooner.

What am I missing? Originally the findSelectedMap(mapID) was embedded in the overlaySelector function but I pulled it out to see if it would help, and it seemed like cleaner coding. All the variables are declared outside the functions. I don't imagine I'm approaching the problem in the best way, but I need the url and zoom (and eventually some other data related to the map).

Greg
  • 2,359
  • 5
  • 22
  • 35
  • You probably call `overlaySelector()` before, which contains a `change` event. So of course once that `change` event on `#select-overlay` is fired and intercepted the code will execute before your `changeLayerTo` is assigned. `$.getJSON` is asynchronous too, best to keep that in mind. – Adrian Dec 31 '17 at 01:14
  • functions don't fire until they are called. – Ronnie Royston Dec 31 '17 at 01:17
  • 1
    Async call is your problem here. While `$.getJSON` is being executed you're immediately assigning currentLayer. – Ele Dec 31 '17 at 01:17
  • `overlaySelector()` is called when the page first loads. – Greg Dec 31 '17 at 01:17
  • @Eleazar Enrique. I see your fix below. I'll give it a try. I think I understand the asynchronous part, but don't understand the work around. But I'll give it a try. Could I put something wrapping the $.getJSON to tell it to wait? That just seems more direct to me. I'm a JavaScript newbie. @Adriani6: I can't run the `overlaySelector()` after since I pick up the selection in there. But maybe I'm not getting what you're saying. I'll be back when I've tried the suggestion. I deleted a lot of script in the posting, so want to take it slowly. Thank you all – Greg Dec 31 '17 at 01:34
  • @Pointy: It may be a duplicate, but I didn't know I had an asynchronous problem since I'm too new to know that was the issue. And the answer you gave didn't show up as a suggestion based on what I asked. In other words how would I have found the answer? I mean that to be an honest question. – Greg Dec 31 '17 at 01:40
  • @Eleazar Enrique. Can't seem to edit my comment above. It works and I see what it is doing. (I still can't get my head around passing functions back and forth. What you did makes sense, but I'd never come up with it.) I'll look at getting rid of some of the global variables. Most were introduced in trying to debug this and other problems. And I still have to finish integrating the fix because the `more map stuff` needs to get moved now. But I think I can work through that. Thank you and @adriani6 for the quick answers. – Greg Dec 31 '17 at 01:50
  • @Greg it's not a negative judgment to have a question closed as a duplicate, for exactly the reason you state: until you've had experience with less-than-obvious things like JavaScript asynchronous APIs, you don't have any way of knowing what you're dealing with. – Pointy Dec 31 '17 at 13:34

1 Answers1

2

Try the following:

function findSelectedMap(mapID, cb) {
    $.getJSON('maps.json', function (json) {
        json.forEach(function (entry) {
            if (entry.maps.id == mapID) {
                changeLayerTo = entry.maps.url;
                maxZoom = entry.maps.zoom;
            }  // end if
        }); // end json.forEach

        cb();
    });    // end $.getJSON
}       // end findSelectedMap

function overlaySelector() {
    $("#select-overlay").change(function () {
        mapID = $("#select-overlay input[type='radio']:checked").val();
        findSelectedMap(mapID, function() {
            currentLayer = L.tileLayer(changeLayerTo).addTo(map);
        }); // using mapID, find the url, zoom for overlayMap selected            
        // more map stuff
    }); // end $( "#select-overlay" ).  
} // end overlaySelector function

Basically, the cb param is a callback function that will be executed just after forEach loop ends and then this line currentLayer = L.tileLayer(changeLayerTo).addTo(map); will use the correct changeLayerTo.

UPDATE - As @Adriani6 suggests: You should pass the variables back via the callback rather than assigning them globally to the window object, of course, if they don't need to be global

Ele
  • 33,468
  • 7
  • 37
  • 75
  • 1
    If that's the case, you should pass the variables back via the callback rather than assigning them globally to the window object, of course if they don't need to be global. But that's a minor suggestion. – Adrian Dec 31 '17 at 01:22
  • @Adriani6 you're right, however, I don't have more information about the code's flow. – Ele Dec 31 '17 at 01:24
  • Correct, that's why it's a minor suggestion, meaning I would put that as part of the answer (as a suggestion obviously for OP). – Adrian Dec 31 '17 at 01:26