Merging JSON and inserting into the DOM

Question

I'm an aspiring junior dev. This is modified version of the code I wrote for a technical interview I did today (Didn't pass. Sad.). The problem was to fetch two json files, and display their data on a table. No JQuery, only a simple un-editable html file.

I'm looking for:

• Ways I could have made the code more readable, improved variable names, commenting.
• Security problems with injecting data into the DOM
• Is the pattern of trying to normalize the data correct?
• Is it good to curry functions like this?
• Should I have memoized the user data in a store?
• (Only if it's allowed on CR) Tips for an interview question like this.

const apiAddress = 'https://api.myjson.com/bins/'
const tableSchema = {short_name: 'User', logins: 'Logins', attempts: 'Attempts'}
// JSON data stored at
// https://api.myjson.com/bins/14pl91
// https://api.myjson.com/bins/xkdzp
/**
 * Fetch should be polyfilled if supporting older browsers
 * 
 * (String) => Object
 * @param {String} path The localpath and filename of the json file
 * @returns {Object}
 */
const fetchData = (path) => {
 return fetch(apiAddress + path)
 .then(res => {
 return res.json()
 }).catch(console.error)
}
/**
 * Use this: https://github.com/paularmstrong/normalizr instead
 * 
 * (String) => (Array) => Object
 * @param {string} key the key used to tell if different objects are representing the same thing
 * @param {[Object<{key: String}>]} objectArray an array of objects, each object must contain the key as a property
 * @returns {Object<{[key: String]: Object}>} Returns an object that contains the input objects mapped by the key
 */
const normalizer = (key) => (objectArray) => {
 return objectArray.reduce((acc, cur) => {
 if (cur.hasOwnProperty(key)) {
 return Object.assign(acc, {[cur[key]]: {...cur}})
 }
 }, {})
}
/**
 * ([Object]) => Object
 * @param {[Object<{[key: String]: Object}>]} objectArray An array of keyed (normalized) objects that will be merged by said key
 * @returns {Object<{[key: String]: Object}>} The merged object
 */
const mergeObjects = (objectArray) => {
 return objectArray.reduce((acc, cur) => {
 Object.keys(cur).map((key) => {
 acc[key] = Object.assign(cur[key], acc[key])
 })
 return acc
 }, {})
}
/**
 * Should place these table functions within a class, prototypes, component
 * 
 * (schema: Object<ITableSchema>) => Table
 * @param {Object<ITableSchema>} schema The schema the table will conform to
 * @returns {Table} A new table instance
 */
const createTable = (schema) => {
 const table = document.createElement('table')
 const headerRow = table.createTHead().insertRow()
 
 // Add the titles in each column
 obj2Arr(schema).forEach((title) => {
 const th = document.createElement('th')
 th.innerText = title
 headerRow.appendChild(th)
 })
 
 return table
}
/**
 * (table: Table, fields: [String]) => (entry: Object<any>) => void
 * @param {Table} table Adds rows to a table reference
 * @param {[String]} fields Fields is an ordered array of keys to get from the entry data
 * @param {Object<any>} entry Data that is to be inserted into the row cells
 */
const createRowInTable = (table, fields) => (entry) => {
 const row = table.insertRow()
 fields.forEach((field) => {
 const cell = row.insertCell()
 cell.innerText = entry[field]
 })
}
/**
 * Utils
 * Makes everything below a bit cleaner and easier to follow
 * Would like to place these in separate files or use lodash
 *
 * @param {Object} object A mapped object
 * @returns {Array} An array without mappings
 */
const obj2Arr = (object) => Object.keys(object).map((key) => object[key])
/**
 * Filters objects that don't contain all keys
 *
 * @param {[String]} keys Keys to check exist in the object
 * @param {Object} object Object that will be checked contains keys
 * @returns {Boolean}
 */
const objFilter = (keys, object) =>
 keys.reduce((acc, cur) => {
 return acc ? object.hasOwnProperty(cur) : false
 }, true)
/**
 * init() main() <- for people that cmd+f
 */
Promise.all([
 fetchData('14pl91'),
 fetchData('xkdzp')
]).then(res => {
 // Manually strip the response, ugly but can't think of a better way
 const strippedResponse = res.map((ele) => {
 if (ele.hasOwnProperty('users')) return ele.users
 if (ele.hasOwnProperty('user_stats')) return ele.user_stats
 })
 // Process the response
 const normalizedUsers = strippedResponse.map(normalizer('id'))
 const users = mergeObjects(normalizedUsers)
 // Filters users without the requisite fields
 const columnFilter = objFilter.bind({}, Object.keys(tableSchema)) // To show PA instead of currying
 const filteredUsers = obj2Arr(users).filter(columnFilter)
 // Build the table
 const table = createTable(tableSchema)
 
 // Put the filtered users inside the table
 const createUserRow = createRowInTable(table, Object.keys(tableSchema))
 filteredUsers.forEach(createUserRow)
 // Find the table's injection point
 const container = document.getElementsByClassName('json-table')[0]
 .lastElementChild
 .lastElementChild
 // Place table inside DOM
 container.appendChild(table)
})

<!DOCTYPE html>
<html>
<head>
 <script src="script.js"></script>
</head>
<body>
 <div class="json-table">
 <h3>JSON Table</h3>
 <div class="module-border">
 <div class="module-content">
 </div>
 </div>
 </div>
</body>
</html>

Note: I've removed all references to the company and mutated the data while keeping the structure and problem identical.

EDIT

Question Specification

• Fetch the contents of the JSON users.json and stats.json
• The purpose of the table is to show user stats. If a user does not have stats, don't show them.
• Parse the JSON files and display their data using a table as shown below, using the short_name, logins, and attempts fields.

• Blindman67: A quick look at the data shows that two records are incomplete. I am sure the interview question would have included details regarding what to do with missing or malformed data. This makes your question ambiguous and hard to answer. It would help someone answering this question if you included the original interview question (in part or full).
• slashdotrules: @Blindman67 Added the question specification. When I run the snippet it returns the correct table (There was a screenshot in the spec)
• msanford: Teeny-tiny style point: when an arrow function takes exactly one argument, the parenthesis are optional; i.e., .map((key) => object[key]) can be written .map(key => object[key]) which some people find more readable.
• Tom Siwik: Are you familiar with ES6' spread operator? If you are seeking idiomatic code this can bring you to the next level. Some functions are just not needed Object.values() gives you what you do withobj2Arr
• slashdotrules: @insertusernamehere for all intents and purposes that is the markup used in the question. Strictly not to touch it.

Answer 1

const fetchData = (path) => {
 return fetch(apiAddress + path)
 .then(res => {
 return res.json()
 }).catch(console.error)
}

The problem with this is that the caller of fetchData will not be aware of the error, since catch did its job - catch the error. Don't catch, and if it's for logging, catch and rethrow. Allow the caller to deal with the rejection.

---

// Manually strip the response, ugly but can't think of a better way
const strippedResponse = res.map((ele) => {
 if (ele.hasOwnProperty('users')) return ele.users
 if (ele.hasOwnProperty('user_stats')) return ele.user_stats
})

This makes the code aware of the presence of "users" and "user_stats". You could push this off to the part of the code that was still specific, moving it away from the more generic parts. For instance, tack on another then on the fetch that extracts the property.

---

// Process the response
const normalizedUsers = strippedResponse.map(normalizer('id'))
const users = mergeObjects(normalizedUsers)

You did not have to normalize both arrays. What ended up happening was you turned an array to an object, but then unpacked the object into an array with mergeObjects. Lots of packing and unpacking going on. You could have turned only one of them as a lookup, while you map through the other.

---

// Build the table
const table = createTable(tableSchema)
// Put the filtered users inside the table
const createUserRow = createRowInTable(table, Object.keys(tableSchema))
filteredUsers.forEach(createUserRow)
// Find the table's injection point
const container = document.getElementsByClassName('json-table')[0]
 .lastElementChild
 .lastElementChild
// Place table inside DOM
container.appendChild(table)

This was entirely unnecessary. True, this would be The Right WayTM to do it in JS, but since the requirement was for one-time render, you could have just constructed the table as a string and used innerHTML.

---

In summary, nothing is wrong with the code. This is what I would have done initially, laying out the things as them come into mind (the grouping, the merging, the filtering, the rendering). It's when you take a second look, and once you have taken into account other factors, is when you start to optimize.

const container = document.getElementById('stats')
const users = fetch('https://api.myjson.com/bins/14pl91').then(r => r.json()).then(r => r.users)
const userStats = fetch('https://api.myjson.com/bins/xkdzp').then(r => r.json()).then(r => r.user_stats)
Promise.all([users, userStats]).then(([users, userStats]) => {
 // Convert stats into a lookup
 const statsLookup = userStats.reduce((c, v) => (c[v.id] = v, c), {})
 const userData = users
 // Remove those users with no stats
 .filter(u => statsLookup.hasOwnProperty(u.id))
 // Grab the data we need
 .map(u => ({
 short_name: u.short_name,
 logins: statsLookup[u.id].logins,
 attempts: statsLookup[u.id].attempts
 }))
 // Render
 container.innerHTML = `
 <table border="1">
 <thead>
 <tr>
 <th>User</th>
 <th>Logins</th>
 <th>Attempts</th>
 </tr>
 </thead>
 <tbody>
 ${userData.map(u => `
 <tr>
 <td>${u.short_name}</td>
 <td>${u.logins}</td>
 <td>${u.attempts}</td>
 </tr>
 `).join('')}
 </tbody>
 </table>
 `
}).catch(e => {
 // Something went wrong. Do something.
})

<div id="stats"></div>

• Kruga: The problem with .innerHTML is if any of the data looks like HTML, it will be parsed as HTML. This will ruin your table, and potentially mess up the entire page. You can't be sure someone won't have a strange name. Someone could also insert a script tag in the data and run all kinds of code.
• Joseph: @Kruga What the interviewer is usually after with interview code submissions is to know how familiar the candidate is with the language, how they write code, and the approach to solving it. You can add all the security you want, but at the end of the day, the farthest this code is ever going to reach is the interviewer's text editor.