Fetching and parsing JSON data from the Oodle API

Question

Here is a working proof of concept in vanilla JS that uses the fetch API to retrieve and parse JSON data. Coming from a server-side background, I wanted insight as to where my beginner JS skills could be improved. Namely, with looping through results, error detection/display, and escaping 'bad data' from third parties before displaying on screen.

    <!doctype html>
<html lang="en">

<head>

    <meta charset="utf-8">
    <meta http-equiv="x-ua-compatible" content="ie=edge">
    <meta name="viewport" content="width=device-width, initial-scale=1, viewport-fit=cover">
    <title>Javascript Experiments - fetch(ing) Data from the Oodle API</title>
    <style>body {font-family:'Oxygen', sans-serif;font-size:14px;line-height:18px;color:#252525;font-weight:400;}</style>

    <body>

        <h1>Javascript Experiments - fetch(ing) Data from the Oodle API</h1>

        <form>
            <input id="st" type="text">
             <select id="numrequests">
                <option value="5" selected>5</option>
                <option value="10">10</option>
                <option value="15">15</option>
                <option value="20">20</option>
                <option value="25">25</option>
            </select>
            <input type="submit" value="Search It Up!">
        </form>

        <p id="result"></p>

        <script>
            document.querySelector('form').onsubmit = function(e) {

                e.preventDefault()

                let oodleurl, txt='', noimage = '<img src="img/noimage.gif">'

                let searchterm = document.getElementById('st').value.trim()
                let sel = document.getElementById('numrequests')
                let numreq = sel.options[sel.selectedIndex].value

                let p = {
                        category: 'vehicle/parts',
                        oodlepartnerid: 'TEST',
                        oodleexclude: 'ebay',
                        searchterm: searchterm,
                        numreq: numreq
                };

                oodleurl = 'https://api.oodle.com/api/v2/listings?key=' + p.oodlepartnerid + '&region=usa&q=' + encodeURIComponent(p.searchterm) + '&category=' + p.category + '&num=' + p.numreq + '&sort=ctime_reverse&exclude_sources=' + p.oodleexclude + '&format=json&jsoncallback=none'

                fetch(oodleurl)
                .then(resp => {
                    if (resp.status === 200) {
                        return resp.json()
                    } else {
                        throw new Error('There was a problem with the API request.')
                    }
                })
                .then(resp => {

                    if (Array.isArray(resp.listings) && resp.listings.length) {

                        resp.listings.forEach(function(v, i) {
                            if (Array.isArray(v.images) && v.images.length) {
                                txt += (v.images[0].hasOwnProperty('src') ? '<img src="' + v.images[0].src + '"><br>' : noimage + '<br>')
                            } else {
                                txt += noimage + '<br>'
                            }
                            txt += 'TITLE: ' + (v.hasOwnProperty('title') && v.hasOwnProperty('url') ? '<a href="' + escapeOutput(v.url) + '" target="_blank">' + escapeOutput(v.title) + '</a>' : 'n/a') + '<br>'
                            txt += 'BODY: ' + (v.hasOwnProperty('body') ? escapeOutput(v.body) : 'n/a') + '<br>'
                            txt += 'CITY: ' + (v.location.hasOwnProperty('name') ? escapeOutput(v.location.name) : 'n/a') + '<br>'
                            txt += 'STATE: ' + (v.location.hasOwnProperty('state') ? escapeOutput(v.location.state) : 'n/a') + '<br>'
                            txt += 'COUNTRY: ' + (v.location.hasOwnProperty('country') ? escapeOutput(v.location.country) : 'n/a') + '<br>'
                            txt += 'PRICE: ' + (v.attributes.hasOwnProperty('price_display') ? escapeOutput(v.attributes.price_display) : 'n/a') + '<br>'
                            txt += 'POSTED: ' + (v.hasOwnProperty('ctime') ? epochToDate(v.ctime) : 'n/a') + '<br>'
                            if (i+1 !== resp.listings.length) txt += '<hr>'
                        })

                        return document.getElementById('result').innerHTML = txt

                        //

                    } else {

                        return document.getElementById('result').innerHTML = 'There are no results to display.'

                    }

                })
                .catch(function(error) {
                    console.log('error', error)
                })

            }

            function escapeOutput(str) {
                return str.replace('&','&amp;').replace('<','&lt;').replace('>','&gt;').replace('"','&quot;').replace("'",'&#x27').replace('/','&#x2F');
            }

            function epochToDate(epoch) {
                if (epoch < 10000000000) {
                    epoch *= 1000
                    var epoch = epoch + (new Date().getTimezoneOffset() * -1)
                    return new Date(epoch).toLocaleDateString()
                }
            }
        </script>

    </body>

</html>

Sᴀᴍ Onᴇᴌᴀ · Accepted Answer · 2018-02-05 17:03:26Z

Feedback

The code looks like it works well and utilizes the API in a good manner. The form markup looks very simplistic. The JavaScript code uses the fetch API in a good way and handles the image arrays appropriately. As far as tips for looping through the results, see the last suggestion below.

Suggestions

Below are a few tips for cleaning up the Javascript a little. Most of the explanations are based on experience and reading posts like this one.

Form submission handler

The first part of the JavaScript starts by registering the onsubmit property to the form.

document.querySelector('form').onsubmit = function(e) {

This approach works but if there was a need to have multiple functions applied when the form was submitted, that original function would need to be modified or wrapped in another function that would call it. A slightly different approach would be to utilize Element.addEventListener(), passing 'submit' as the eventtype and the function as the listener

document.querySelector('form').addEventListener('submit', function(e) { }
    e.preventDefault();
    //...
);

And instead of using document.querySelector('form') to select the form element, document.forms[0] could be used, document.getElementById() could be used if an id attribute is added to that form element, or even simpler, document.body can be used because the event is bubbled up through the DOM.

document.body.addEventListener('submit', function(e) {
    e.preventDefault();
    //...
);

DOM element references should be cached

The code calls document.getElementById() each time the form is submitted to access the form fields. Instead of those lookups occurring each time the form is submitted, a more efficient approach would be to store those references in a variable outside the form submission handler. Since those don't change, const can be used.

const searchtermElement = document.getElementById('st');
const sel = document.getElementById('numrequests');
const resultContainer = document.getElementById('result');

Then those can be used inside the form submission handler:

let searchterm = searchtermElement.value.trim()
let numreq = sel.options[sel.selectedIndex].value

And actually those variables searchterm, numreq and p don't get reassigned so const can be used for those instead of let.

Form Submit handler return values?

There are a few return statements that return the value of assigning the inner HTML of the result element. That isn't necessary. If anything was returned, I would think a value equivalent to false should be returned, so as to avoid the default form submission handler, but there is already a call to e.preventDefault().

Looping through results

Instead of calling Array.forEach() and appending to txt, Array.map() could be used to get an array of strings for the listing and then Array.join() could be used to add the array elements as the inner HTML of the results container. See this in action in the snippet below. Notice that the function used in the call to .map() is split out to a separate function and resp is not defined there. Fortunately the third argument of the callback to .map() is a reference to the array so it can tell when the last element is processed.

Notice how the use of .map() allows the callback function to be moved outside of the code where it is called. With this approach, there is no access to the variable txt. Thus there is no dependency on that variable. This is loosely one aspect of the dependency injection principle - one of the S.O.L.I.D. principles.

One possible way to simplify that listing HTML code is to use a template literal. I will leave it as an exercise to the reader. Also, the function could return an element (e.g. from document.createElement()) but then the elements would need to be appended to the results container and any existing elements would need to be cleared.

Measuring time taken to retrieve, parse and display results after each form submission

Originally I was thinking of using a function like Date.now() to compute the time before and after the results have been fetched and processed. However, this SO answer suggests using the now standard API performance.now() (unless you need to support older browsers like IE 9 or earlier).

In the snippet below, I added a <div> to hold the time output below the form. Before the fetch() call, there time is stored in a constant:

const startTime = performance.now();

And after the results are processed, that is used to calculate the number of milliseconds:

timeOutputContainer.innerHTML = 'time taken: '+(performance.now() - startTime) + ' ms';

Sanitizing data

I must admit actually haven't dealt with sanitizing data much via JavaScript but I have found various techniques:

While it was written back in 2012, this post claims to have a foolproof technique (and it even discusses the code you have - see Hack #1).

So with that approach, your code could be more like:
```
function escapeOutput(str) {
    var div = document.createElement('div');
    div.appendChild(document.createTextNode(str));
    return div.innerHTML;
}
```
After reading this article it looks like the DOMPurify library can help along with those template literals mentioned above. Try out my jsfiddle after reading that blog post.

const searchtermElement = document.getElementById('st');
const sel = document.getElementById('numrequests');
const resultContainer = document.getElementById('result');
const timeOutputContainer = document.getElementById('timeOutput');

const noimage = '<img src="img/noimage.gif">'
    
document.body.addEventListener('submit', function(e) {
  e.preventDefault()

  const searchterm = searchtermElement.value.trim()
  const numreq = sel.options[sel.selectedIndex].value

  const p = {
    category: 'vehicle/parts',
    oodlepartnerid: 'TEST',
    oodleexclude: 'ebay',
    searchterm: searchterm,
    numreq: numreq
  };

  const oodleurl = 'https://api.oodle.com/api/v2/listings?key=' + p.oodlepartnerid + '&region=usa&q=' + encodeURIComponent(p.searchterm) + '&category=' + p.category + '&num=' + p.numreq + '&sort=ctime_reverse&exclude_sources=' + p.oodleexclude + '&format=json&jsoncallback=none';
  const startTime = performance.now();

  fetch(oodleurl)
    .then(resp => {
      if (resp.status === 200) {
        return resp.json()
      } else {
        throw new Error('There was a problem with the API request.')
      }
    })
    .then(resp => {
      if (Array.isArray(resp.listings) && resp.listings.length) {
        const listings = resp.listings.map(getListingOutput);
        resultContainer.innerHTML = listings.join();

      } else {
        resultContainer.innerHTML = 'There are no results to display.';
      }
      timeOutputContainer.innerHTML = 'time taken: '+(performance.now() - startTime) + ' ms';
    })
    .catch(function(error) {
      console.log('error', error)
    })

});

function getListingOutput(v, i, listings) {
  let listingHTML = '';
  if (Array.isArray(v.images) && v.images.length) {
    listingHTML += (v.images[0].hasOwnProperty('src') ? '<img src="' + v.images[0].src + '"><br>' : noimage + '<br>')
  } else {
    listingHTML += noimage + '<br>'
  }
  listingHTML += 'TITLE: ' + (v.hasOwnProperty('title') && v.hasOwnProperty('url') ? '<a href="' + escapeOutput(v.url) + '" target="_blank">' + escapeOutput(v.title) + '</a>' : 'n/a') + '<br>'
  listingHTML += 'BODY: ' + (v.hasOwnProperty('body') ? escapeOutput(v.body) : 'n/a') + '<br>'
  listingHTML += 'CITY: ' + (v.location.hasOwnProperty('name') ? escapeOutput(v.location.name) : 'n/a') + '<br>'
  listingHTML += 'STATE: ' + (v.location.hasOwnProperty('state') ? escapeOutput(v.location.state) : 'n/a') + '<br>'
  listingHTML += 'COUNTRY: ' + (v.location.hasOwnProperty('country') ? escapeOutput(v.location.country) : 'n/a') + '<br>'
  listingHTML += 'PRICE: ' + (v.attributes.hasOwnProperty('price_display') ? escapeOutput(v.attributes.price_display) : 'n/a') + '<br>'
  listingHTML += 'POSTED: ' + (v.hasOwnProperty('ctime') ? epochToDate(v.ctime) : 'n/a') + '<br>'
  if (i + 1 !== listings.length) listingHTML += '<hr>';
  return listingHTML;
}

function escapeOutput(str) {
    var div = document.createElement('div');
    div.appendChild(document.createTextNode(str));
    return div.innerHTML;
}

function epochToDate(epoch) {
  if (epoch < 10000000000) {
    epoch *= 1000
    var epoch = epoch + (new Date().getTimezoneOffset() * -1)
    return new Date(epoch).toLocaleDateString()
  }
}

<h1>Javascript Experiments - fetch(ing) Data from the Oodle API</h1>

<form>
  <input id="st" type="text">
  <select id="numrequests">
    <option value="5" selected>5</option>
    <option value="10">10</option>
    <option value="15">15</option>
    <option value="20">20</option>
    <option value="25">25</option>
  </select>
  <input type="submit" value="Search It Up!">
</form>
<div id="timeOutput"></div>
<p id="result"></p>

Thanks Sam! Three quick follow up questions: 1. Why map() instead of forEach()? 2. Can escapeOutput() be improved to sanitize unknown third party data? 3. What is best way to show time (in milliseconds or seconds) it takes to retrieve, parse and display results after each form submission? — JSNewbish, Commented Jan 19, 2018 at 14:29
I have updated my answer . For the first question there is now a new paragraph between the existing two under Looping through results, and then new sections for the second and third questions — Sᴀᴍ Onᴇᴌᴀ, Commented Jan 19, 2018 at 17:17
@JSNewbish if this answer helped you, it would be good to mark it as the Accepted Answer! ;-) — janos, Commented Dec 17, 2018 at 17:07

Stack Exchange Network

Fetching and parsing JSON data from the Oodle API

1 Answer 1

Feedback

Suggestions

Form submission handler

DOM element references should be cached

Form Submit handler return values?

Looping through results

Measuring time taken to retrieve, parse and display results after each form submission

Sanitizing data

Hot Network Questions

Fetching and parsing JSON data from the Oodle API

1 Answer 1

Feedback

Suggestions

Form submission handler

DOM element references should be cached

Form Submit handler return values?

Looping through results

Measuring time taken to retrieve, parse and display results after each form submission

Sanitizing data

Related

Hot Network Questions