Closed. This question is . This question is opinion-based . It is not currently accepting answers. Want to improve this question? Update the question so it can be answered with facts and citations by editing this post. Closed 3 years ago. Improve this question

I'm a Sr. front-end dev, coding in Babel ES6. Part of our app makes an API call, and based on the data model we get back from the API call, certain forms need to be filled out.

Those forms are stored in a doubly-linked list (if the back-end says some of the data is invalid, we can quickly get the user back to the one page they messed up and then get them back on target, simply by modifying the list.)

Anyway, there's a bunch of functions used to add pages, and I'm wondering if I'm being too clever. This is just a basic overview - the actual algorithm is much more complex, with tons of different pages and page types, but this'll give you an example.

This is how, I think, a novice programmer would handle it.

export const addPages = (apiData) => { let pagesList = new PagesList(); if(apiData.pages.foo){ pagesList.add('foo', apiData.pages.foo){ } if (apiData.pages.arrayOfBars){ let bars = apiData.pages.arrayOfBars; bars.forEach((bar) => { pagesList.add(bar.name, bar.data); }) } if (apiData.pages.customBazes) { let bazes = apiData.pages.customBazes; bazes.forEach((baz) => { pagesList.add(customBazParser(baz)); }) } return pagesList; }

Now, in order to be more testable, I've taken all those if statements and made them separate, stand alone functions, and then I map over them.

Now, testable is one thing, but so is readable and I wonder if I'm making things less readable here.

// file: '../util/functor.js' export const Identity = (x) => ({ value: x, map: (f) => Identity(f(x)), }) // file 'addPages.js' import { Identity } from '../util/functor'; export const parseFoo = (data) => (list) => { list.add('foo', data); } export const parseBar = (data) => (list) => { data.forEach((bar) => { list.add(bar.name, bar.data) }); return list; } export const parseBaz = (data) => (list) => { data.forEach((baz) => { list.add(customBazParser(baz)); }) return list; } export const addPages = (apiData) => { let pagesList = new PagesList(); let { foo, arrayOfBars: bars, customBazes: bazes } = apiData.pages; let pages = Identity(pagesList); return pages.map(foo ? parseFoo(foo) : x => x) .map(bars ? parseBar(bars) : x => x) .map(bazes ? parseBaz(bazes) : x => x) .value }

Here's my concern. To me the bottom is more organized. The code itself is broken into smaller chunks that are testable in isolation. BUT I'm thinking: If I had to read that as a junior developer, unused to such concepts as using Identity functors, currying, or ternary statements, would I be able to even understand what the latter solution is doing? Is it better to do things the "wrong, easier" way sometimes?