-1

I have a function renderActualChart() that renders a chart on my screen.

That requires an input with all the chart settings, since each chart is dynamic based on the value selected from the table to chart i call a forEach loop on the promise i get back from Firebase with the user's data.

I need renderLabsCharts(doc) to run all of its loops FIRST... then i want renderActualChart() to run since the data will be ready at that point.

I have tried using a counter to only run that function on the last loop iteration but when i run in browser it still hits every single forEach loop, i don't get it....

var itemsProcessed = 0;
var size ;
function getDataFromFirebase(){

    console.log('uid before results call: ' + userUid)


db.collection('lab-results')
.where("uid", "==", userUid)
.orderBy("labTestDate", "desc")
.get()
.then((snapshot) => {
    size = snapshot.size;
    snapshot.docs.forEach(doc => {

        itemsProcessed++;
        console.log('itemProcessed: ' + itemsProcessed);
        console.log('size: ' + size)

        console.log(doc.data());


        //Render results for tables and dropdowns on screen
        renderLabResults(doc);
        renderLabResultDeleteList(doc);

        var el = document.getElementById('chart-form');
        if(el){
            //Only render the chart when the drop down is changed
            el.addEventListener("change", function() {
                console.log(' YYYYYY RENDER LABS CALLED YYYYYYYY')
                //Call builder to create the input file to build the chart with the selected lab data
                renderLabsCharts(doc);

                if(itemsProcessed === size){
                    console.log('XXXXXXXX LAST ITEM XXXXX')
                    //Call the code to render the chart, which needs the output from renerLabsCharts that contains the data values for the chart
                     renderActualChart();
                }else{
                    console.log('WHY AM I NEVER HITTING HERE?' + itemsProcessed + ' ' + size)
                }

        });

        }

    })

})

////////////////////////////////////////////////////////////////////////////
// CHARTS
//////////////////////////////////////////////




var labelsString = ''
var dataString = ''
var chartLabel = ''

const chartForm = document.querySelector('#chart-form');


function renderLabsCharts(doc){

    var labTime = doc.data().labTestDate;
     var labTimeDate = labTime.toDate();
     var clearTestDate = moment(labTimeDate).format('DDMMMYYYY');

 console.log('selected value for chart: ' + chartForm.labsLoggedList.value)
 console.log('labTestName: '+ doc.data().labTestName) 


 if(doc.data().labTestName == chartForm.labsLoggedList.value){

    chartLabel = doc.data().labTestName + ' ' + doc.data().labUnits;
    labelsString = labelsString + '"' + clearTestDate + '",';
    dataString = dataString + doc.data().labResult + ',';
    console.log('labelsString: ' + labelsString)
    console.log('dataString: ' + dataString)
    }



}

function renderActualChart(){


    console.log('render chart called')
    console.log('labelsString: ' + labelsString)
    console.log('dataString: ' + dataString)
  //remove comma from front
  //labelsString = labelsString.replace(/(,$)/g, "");
  //dataString = dataString.replace(/,$/g, "");
 // Bar chart
 var chartData = {
    type: 'bar',
    data: {
    //labels: [labelsString],
    labels: ["12DEC2012","11NOV2011"],
    datasets: [
        {
        label: chartLabel,
        backgroundColor: ["#c45850", "#c45850","#c45850","#c45850","#c45850"],
        data: [47,30,100,33,11]
        }
    ]
    },
    elements: {
        rectangle: {
            borderWidth: 2,
        }
    },
    responsive: true,
    options: {
    legend: { display: true },
    title: {
        display: true,
        text: 'Historical Lab Results'
    }
    }
}

    new Chart(document.getElementById("bar-chart-labs"), chartData );

THIS IS WHAT I GET ON INITAL PAGE LOAD

itemProcessed: 1
appLabs.js:188 size: 3
appLabs.js:137 labTime: Timestamp(seconds=1355328660, nanoseconds=0)
appLabs.js:138 labTimeDate: Wed Dec 12 2012 11:11:00 GMT-0500 (Eastern Standard Time)
appLabs.js:143 clearTestDate: 2012-12-12 11:11 AM
appLabs.js:187 itemProcessed: 2
appLabs.js:188 size: 3
appLabs.js:137 labTime: Timestamp(seconds=1355328660, nanoseconds=0)
appLabs.js:138 labTimeDate: Wed Dec 12 2012 11:11:00 GMT-0500 (Eastern Standard Time)
appLabs.js:143 clearTestDate: 2012-12-12 11:11 AM
appLabs.js:187 itemProcessed: 3
appLabs.js:188 size: 3
appLabs.js:137 labTime: Timestamp(seconds=1321027860, nanoseconds=0)
appLabs.js:138 labTimeDate: Fri Nov 11 2011 11:11:00 GMT-0500 (Eastern Standard Time)
appLabs.js:143 clearTestDate: 2011-11-11 11:11 AM

THEN WHEN I CHANGE THE DROP DOWN, I ONLY GET THIS

 YYYYYY RENDER LABS CALLED YYYYYYYY
appLabs.js:510 selected value for chart: GLUCOSE
appLabs.js:511 labTestName: GLUCOSE
appLabs.js:519 labelsString: "12Dec2012",
appLabs.js:520 dataString: 11111,
appLabs.js:206 XXXXXXXX LAST ITEM XXXXX
appLabs.js:530 render chart called
appLabs.js:531 labelsString: "12Dec2012",
appLabs.js:532 dataString: 11111,
appLabs.js:201  YYYYYY RENDER LABS CALLED YYYYYYYY
appLabs.js:510 selected value for chart: GLUCOSE
appLabs.js:511 labTestName: SODIUM
appLabs.js:206 XXXXXXXX LAST ITEM XXXXX
appLabs.js:530 render chart called
appLabs.js:531 labelsString: "12Dec2012",
appLabs.js:532 dataString: 11111,
appLabs.js:201  YYYYYY RENDER LABS CALLED YYYYYYYY
appLabs.js:510 selected value for chart: GLUCOSE
appLabs.js:511 labTestName: GLUCOSE
appLabs.js:519 labelsString: "12Dec2012","11Nov2011",
appLabs.js:520 dataString: 11111,111111,
appLabs.js:206 XXXXXXXX LAST ITEM XXXXX
appLabs.js:530 render chart called
appLabs.js:531 labelsString: "12Dec2012","11Nov2011",
appLabs.js:532 dataString: 11111,111111,
sh1hab
  • 661
  • 5
  • 16
user3569450
  • 75
  • 10
  • Youre code seems ok, there is one problem i can think of and it relates to initialization of `size` and `itemsProcessed`. For some reason, you log them and the start of each loop but it doesn't appear in the log you gave us and the bottom of the post... Please verify that `snapshot.size` actually returns a number, and not an array of sizes / a js Obj / some other unexpected value, and also in your code it doesn't show where do you initialize `itemsProccessed`to be 0. Do you do that? you absolutely sure it stays 0 before it gets to that `.then()` callback? – Gibor Aug 04 '19 at 10:44
  • I have edited my initial response with better logs, i am getting all the values back and can watch the loop progress. – user3569450 Aug 04 '19 at 10:48
  • this is weird... could you change the logs to print `itemProcessed` and `size` right before the comparison? also, out of curiosity, if you forEach loop runs on `snapshot.docs`, why do you initialize `size` to be `snapshot.size` and not `snapshot.docs.length`? – Gibor Aug 04 '19 at 10:58
  • When i log before the compare, it comes back as 3 = 3 every time. I am very much a novice so no reason i coded anything specifically, just trying to get this to work. I also tried ```snapshot.docs.length``` and i get 3 back as well. I think what happening is because that call is inside an eventListener for a drop down change, that happens AFTER the function ```getDataFromFirebase()``` which the forEach loop lives in has already fired because that gets called on page load once we verify a user is actually logged in. – user3569450 Aug 04 '19 at 11:01
  • Maybe i am going about this the wrong way.... is there a way to reference the doc i get back from the snapshot after its been returned from the promise in a separate function? I am trying to avoid multiple calls to firebase, i want to hit it once and then have that doc object available to work with in other functions, then i can call it as i please to parse and pull stuff out, like in this case where i need to build a config file for chart data. – user3569450 Aug 04 '19 at 11:10
  • so, you log at the start of the forEach() and the values are different than when you log right before the comparison? This makes 0 sense... just to rule out the "your IDE has gone crazy" option- change the `itemsProcessed++;` to `itemsProcessed = 1;`, you should be going on an infinite loop. if not, I have no idea how to help from here :) – Gibor Aug 04 '19 at 11:12
  • and to answer your question- you can but its not really recommneded. Values coming from async operations should be kept and processed in async operations. you can use async-await - this waits for a certain promise to resolve / reject before continuing on with the code, kind of making the code around the promise act synchronously. The thing is, the function you use the "await" inside have to be asynchronous as well, and in cases like yours it can be a real asynchronous mess to try and make things act synchronously. – Gibor Aug 04 '19 at 11:16
  • I did not hit an infinite loop, but i think i narrowed down the issue. The counter is already at 3 and done when the eventListener for the drop down fires. I am not seeing the counter go through its iterations again, it ONLY does that on page load. Which is weird because somehow the ```renderLabsCharts(doc)``` function is able to loop as proven in the debug logs. Possible for me to share code with you? Maybe then easier to see whats going on? – user3569450 Aug 04 '19 at 11:21
  • ```BEFORE LOOP: 0 189 itemProcessed: 1 190 size: 3 189 itemProcessed: 2 190 size: 3 189 itemProcessed: 3 190 size: 3 203 YYYYYY RENDER LABS CALLED YYYYYYYY 512 selected value for chart: GLUCOSE 513 labTestName: GLUCOSE 521 labelsString: "12Dec2012", 522 dataString: 11111, 206 vals before compare: 3 3 ``` – user3569450 Aug 04 '19 at 11:24
  • oh wow I completely missed the event listener registration... you're completely right. To solve this, extract the whole function you pass as a callback to the `addEventListener` to a normal, named function with 1 argument- itemsProcessed. Then, inside the loop, do this: `el.addEventListener("change", youFunc.bind(itemProcessed));` this should work. If you need more detailed answer I'll write it on a new answer. – Gibor Aug 04 '19 at 11:40
  • If you could mock up some code i would really appreciate it, i still have a hard time understanding the conceptual stuff but i can work things out if i play around with code example. Thank you very much for you time and help in solving this. – user3569450 Aug 04 '19 at 11:43
  • Here is my attempt at solving, i get back a blank object instead of a number in console: https://pastebin.com/EKHvuRyt – user3569450 Aug 04 '19 at 11:47
  • You can't pass the variable like this to a callback. a callback is always pre-defined according to what the function that gets it (`addEventListener` in your case) expects to get. Think about it- it doesn't know if your function will have 1, 2 or 1000 arguments... you have to bind it. see my answer below and let me know if it worked :) – Gibor Aug 04 '19 at 12:01

2 Answers2

0

because of closure itemsProcessed inside callback function of change event always returns the last value of itemsProcessed in the loop,

pass itemsProcessed into a closure could solve problem

if(el){
    (function(index) {
      el.addEventListener("change", function() {
         console.log(' YYYYYY RENDER LABS CALLED YYYYYYYY')
          //Call builder to create the input file to build the chart with the selected lab data
           renderLabsCharts(doc);

           if(index=== size){
            console.log('XXXXXXXX LAST ITEM XXXXX')
            //Call the code to render the chart, which needs the output from renerLabsCharts that contains the data values for the chart
                         renderActualChart();
            }else{
                        console.log('WHY AM I NEVER HITTING HERE?' + itemsProcessed + ' ' + size)
           }

     });
    }(itemsProcessed)
}
cuongtd
  • 2,862
  • 3
  • 19
  • 37
  • I tried this, but it results in the same problem. renderActualChart() runs for every iteration of the forEach loop. – user3569450 Aug 04 '19 at 10:53
  • I am EXTREMELY green with javascript.... can you show me an example please? – user3569450 Aug 04 '19 at 10:56
  • 1
    @GeorgeJempty second argument is not length but index – cuongtd Aug 04 '19 at 10:59
  • 1
    @GeorgeJempty the 2nd optional argument for the forEach is index, and the 3rd is the array it acts upon. He does not get the size. He could extract the 3rd argument and initialize it to be `array.length`, or simply and more intuitively initialize it before the loop to be `snapshot.docs.length` – Gibor Aug 04 '19 at 11:00
  • I tried this but i get back ```[object Event] ``` in console instead of the number for ```itemsProcessed``` when i pass it into the function. – user3569450 Aug 04 '19 at 11:29
  • @user3569450 make you passed itemsProcessed instead of event into function – cuongtd Aug 04 '19 at 12:40
0

So the problem is, like you said, that the compatison takes place only after the loop is done, because drop-down change is being done at a later time and itemsProcessed is still in the same context it was when the loop ran for the last times- e.g always equals to size.

So, to solve this, first take the whole call-back function you pass to addEventListener outside:

function dropdownChangeCallback(itemsDone){
    console.log(' YYYYYY RENDER LABS CALLED YYYYYYYY')
    renderLabsCharts(doc);

    if(itemsDone === size){
       console.log('XXXXXXXX LAST ITEM XXXXX')
       renderActualChart();
    }else{
       console.log('WHY AM I NEVER HITTING HERE?' + itemsDone + ' ' + size)
}

Then, inside your loop (and inside the if(el)), you use bind to pass the value you need, like this:

el.addEventListener("change", dropdownChangeCallback.bind(itemsProcessed));

When the callback is called, it will use the value of the binded variable (in this case- itemsProcessed) that it had at the time of binding.

To read more about binding, context and more- you can refer here: Bind variables to callback function, or here for easier-to-read info including some extra stuff that are important to know about JS functions.

Gibor
  • 1,695
  • 6
  • 20
  • so close.... now its angry that doc is missing, i am guessing this needs to be passed in or bound somehow like we are passing in itemsProcessed?```doc is not defined at Number.dropdownChangeCallback``` ```function dropdownChangeCallback(itemsDone){ console.log(' YYYYYY RENDER LABS CALLED YYYYYYYY') renderLabsCharts(doc);``` https://pastebin.com/Y8DpYcgG – user3569450 Aug 04 '19 at 12:07
  • well yeah you can bind `doc` too and pass it as well. But you're already inside that forEach loop going on each doc, if you can take the `renderLabsCharts(doc)` outside of that function (maybe just inside the `if(el)` is good?) it would be better practice. otherwise go with that, i'm pretty sure there is a better way to do it but i'd have to know much more of your code to be able to tell you if its true and i'm pretty sure it ain't gonna matter much (unless doc is a really huge data structure, in which case binding too many of those can potentially cause memory problems). – Gibor Aug 04 '19 at 12:14
  • also, glad to help :) if it works for you don't forget to tick this as 'acepted answer' :) – Gibor Aug 04 '19 at 12:15