Random Quote Machine: JS Promises help

Random Quote Machine: JS Promises help
0

#1

Hi guys,
I got the quote machine working fine using data I stored in a JSON file.
However, I realised that an AJAX request was being made every time an user clicks to load a new quote. This would have serious performance implications.
My idea was to somehow store the result of retrieving the JSON data in an array or object, which would then be accessed when the user asks for a new quote.
And here is where asynchronous js punched me in the face for the first time!

I have been doing some reading and watching on ES6 promises, and have managed to get the quote generator working but only on page reloads. The code no longer works when trying to generate a quote by clicking the button.

Here is the JS


// Revealing module pattern
const getQuote = (() => {
  // cache ids
  const quoteButton = document.getElementById('quoteButton');
  const quoteBoxContent = document.getElementById('quotebox--quote-js');
  const quoteBoxAuthor = document.getElementById('quotebox--author-js');

  // create a random number to be used when retriving quote index
  const random = (numOfQuotes) => {
    const x = Math.floor(Math.random() * (numOfQuotes));
    return x;
  };


  function promisedQuote(url) {
    return new Promise((resolve, reject) => {
      const xhr = new XMLHttpRequest();
      xhr.onload = function() {
        resolve(this.responseText);
      };
      xhr.onerror = reject;
      xhr.open('GET', url);
      xhr.send();
    });
  }

  promisedQuote('./data/quotes.json')
  /* here is where I begin to get confused. I'm not sure how to take 
      data out of here to be used elsewhere. I want to store the retrieved
      in a variable so that I dont need an ajax request every time i want a quote.
  */

  .then((result) => {
    const quotes = JSON.parse(result); // quotesExist = true
    const num = random(quotes.quotes.length);
    const quote = quotes.quotes[num].quote;
    const author = quotes.quotes[num].author;
    quoteBoxContent.innerHTML = JSON.stringify(quote);
    quoteBoxAuthor.innerHTML = JSON.stringify(author);
    return quotes;
  })
  .catch((err) => {
    console.log(`[Error]: ${err}`);
  })


  // explicitly return public methods when this object is instantiated
  return {
    quote: promisedQuote,
  };
})();

function swapQuotes(){
  return getQuote;
}
quoteButton.addEventListener('click', swapQuotes());

Here is the dev branch where you can see my code.

Here is the previous working version: https://thiagodebastos.gitlab.io/fcc-random-quote-generator/


#2

I don’t understand why you need promises when you can use xhr.onload to assign a function that selects and displays the quote. Personally, I would only use promises if more than one asynchronous operation had to be executed synchronously.

While I don’t know if my approach clashes with your pattern, I’d wrap your code in a function and use a closure to store the xhr results. In promisedCode, I’d check for their existence and either return a promise doing the xhr or one that immediately resolves.

I hope that is helpful :slight_smile:


#3

Hi Lin,
thanks so much for your response. Originally I had it working without any promises, but I found that it triggers a HTTP request every time I want to generate a new quote and thought that I could improve performance by doing that only once and caching the results into a variable. If I declare that variable outside of promisedQuote(), the variable will always return undefined due to the request to json being asynchronous.

If I understand what you are saying, I should enclose promisedQuote()? I’m very new with using AJAX, this is pretty much my first time! I’ll do some more research on the .onload property, perhaps I’m missing some rather basic AJAX concept.

Again, thank you! I will let you know how I go :slight_smile:


#4

Hey there,

There are a few points to consider here.

  • Promises by themselves are not going to save you any HTTP calls. There are a lot of benefits to them, but this is not one.
  • Some of the revealing pattern you’re implementing is unnecessary, but you’re on the right track.
  • You want to call the promise from the button’s click handler, not inside of the revealed module

I made this pen to show how to use a promise to make an XHR call each time (I didn’t write a single XHR call, you’ll have to use your imagination)

http://codepen.io/PortableStick/pen/gMmxVJ

Here’s the same idea, but caching the results from the promise, which is what you’re trying to accomplish

http://codepen.io/PortableStick/pen/yJMzXo?editors=1010

I tried to make the comments describe the code so I wouldn’t have to write a long thing here. Notice that in the first, comments from the AJAX call happen each time you press the button. Not so with the second.


#5

Thank you so much my friend. This actually helped me quite a bit with the weather app!


#6

I just thought I’d show you my weather app! Your explanations really helped, thank you :slight_smile:

Preview: https://thiagodebastos.gitlab.io/fcc-weather-app/
Code: https://gitlab.com/thiagodebastos/fcc-weather-app

So I gather that the only way to save on external ajax calls would be to fetch the json and in the backend, store that data in a local file/database?


#7

It looks really great! One problem:

Yup, so long as by “backend” you don’t mean a server that you’d have to make another call to. The basic idea is to cache the data somehow and then perform other manipulations on it as necessary.


#8

Oh thanks for pointing that out! Initially, if the property existed it would display a decimal value, or if not “no rain”. Until this:

When I converted the result to a percentage I forgot that the property might not exist, thus NaN!


#9

That’s what I mean. Would you store it to local storage? I am a bit confused about how to separate the ajax call function from a function that would access the cached data. Perhaps I would need some form of PubSub implementation like React or Meteor does.


#10

Local storage is a good choice if you need the data to persist after the user leaves the page. I’m not at all familiar with Meteor, but the purpose of the Flux architecture is more about moving the flow of data in one predictable way than simply storing data. Whatever you’re caching could simply be placed in a variable and you’ll be able to manipulate it at your leisure. It’s no different from having a hard coded collection of values.


#11

The thing is, from what I understand so far, when I store it in a variable that exists outside of the function’s scope, say in a global variable, and then try to access the data in that variable from outside the function enclosing the ajax request, the result will always be undefined, because js will try to access the results from that variable instantly, at which point ajax is not ready. This is what leads me to think that I might need a function that subscribes to changes in a variable, so that when its content changes, the function fires?
The only other way is to call the function that needs the data as a callback from the ajax function but then we are back at square one, we still need to call the ajax function. Confusion!


#12

Oh yeah, of course! You’ll always have to be conscious of race conditions and scope. PubSub is certainly one solution, but it’s not necessarily the only one. Closures are common in D3, promises work well for Node, and generator functions let us reason about asynchronous code synchronously. Right now, there are no perfectly satisfactory ways of dealing with this is a browser, so you’ll have to weigh your options with what you have. Promises are generally a well supported way of caching data from an HTTP call, but that means writing a lot of code inside your .then() function.


#13

I sure look forward to getting my head around all of these options. All of this has opened up a whole new world for me and I am having so much fun in the process! I really look forward to getting my hands on D3!