2
\$\begingroup\$

I have this array const idArray = ["12", "231", "73", "4"] and an object

const blueprints = {
  12: {color: red, views: [{name: "front}, {name: "back}, {name: "top}, {name: "bottom}]},
  231: {color: white, views: [{name: "front}, {name: "back}]},
  73: {color: black, views: [{name: "front}, {name: "back}, {name: "top}, {name: "bottom}]},
  4: {color: silver, views: [{name: "front}, {name: "back}, {name: "top}, {name: "bottom}]},
}

How can I return an array of the following objects that have all front, back, top, and bottom using ES6 map/filter/some and etc?:

result =[
    {colorId: "12", views: [{name: "front}, {name: "back}, {name: "top}, {name: "bottom}]}
    {colorId: "73", views: [{name: "front}, {name: "back}, {name: "top}, {name: "bottom}]}
    {colorId: "4", views: [{name: "front}, {name: "back}, {name: "top}, {name: "bottom}]}
]

I did it here but I feel like it is too messy and hard to read. Anybody could recommend on how to shorten it and make it easier to read using the ES6 functions (map, filter ...)?

const result = idArray.map(id => {
      const bluePrint = bluePrints[id];
      const exists = bluePrint.views.some(view => view.name === 'top' || view.name === 'bottom');

      if (exists) {
        return {
          colorId: id,
          views: bluePrint.views
        }
      }
    }).filter(bluePrint => bluePrint);
\$\endgroup\$
2
  • 2
    \$\begingroup\$ You have some mismatched quotes in your JSON? \$\endgroup\$ – 200_success Nov 15 '18 at 18:49
  • 1
    \$\begingroup\$ Even once you fix the quotes, blueprints causes "Uncaught ReferenceError: red is not defined". Please post usable data. \$\endgroup\$ – Carcigenicate Nov 15 '18 at 19:30
1
\$\begingroup\$

Keeping it simple

One big problem that coders must face is complexity. Complexity is many times the actual result of wanting to simplify code readability, but often this is at the expense of functional and source simplicity.

Complexity propagates throughout a project. In the example of your code the view name objects {name : "top"} means that at all levels of the project the need to access a view by name requires the additional object property reference "name". On a large project the seamingly simple use of a named property can add dozens or more lines of source code.

Simplicity means looking further than just the immediate code, but how data structure, data sources, and access methods effect code complexity throughout the project.

Sources of complexity

  1. Data duplication

You have some data duplication.

The array idArray is a duplication of the blueprints keys and can be computed using Object.keys(bluePrints) or better still use a for loop to extract both the id and value of each entry in bluePrint.

  1. Expression of the implied

The view array has objects containing just one property {name : "bottom"}, ... which just adds complication. Why not just have an array of view names. This will make future manipulation easier.

eg finding a view in array of string is

const printHasView = (print, viewName) => print.views.includes(viewName);

As opposed to the more complicated search that requires an addition call stack item, associated context, and additional references.

const printHasView = (print, viewName) => print.views.some(view => view.name === viewName);
  1. Indirect references

The property colorId is an indirect reference to an object in a, (I am guessing) map, mapLike, or array of colors.

This means that each time you need to handle the print's color you need to include code that locates the color by Id. You can simplify by using a direct reference to the color, you thus don't need to lookup the color by Id each time you need it. Saving functional and source complexity.

Example

This makes some assumptions about your project and code and is only meant as an example. How your project is structured is unknown and will affect the quality of this example.

An alternative blueprint filter as a function that has an argument pertaining to the filter criteria, removing view name object in favour of the simpler string, and directly referencing the colors.

// arg views is array of strings ["top", "bottom"]
const filterPrintsByViews = (prints, ...views) => {
    const result = [];
    for (const [id, value] of Object.entries(prints)) {
        if (views.every(name => value.views.some(view => view.name === name))) {
            result.push({colorRef: colors[id], views: value.views.map(view => view.name)});
        }
    }
    return result;
}

// Data sources ============================================
// assuming that id and value are not uinque
const colors = {
    "12": {color: "red"},
    "231":{color: "white"},
    "73": {color: "black"},
    "4":  {color: "silver"},
    "20": {color: "silver"}, // second case of silver
};

const blueprints = {
  "12":{color: "red", views: [{name: "front"}, {name: "back"}, {name: "top"}, {name: "bottom"}]},
  "231":{color: "blue", views: [{name: "front"}, {name: "back"}]},
  "73":{color: "cyan", views: [{name: "front"}, {name: "back"}, {name: "top"}, {name: "bottom"}]},
  "4":{color: "gold", views: [{name: "front"}, {name: "back"}, {name: "top"}, {name: "bottom"}]},
};


// Usage =========================================================
const result = filterPrintsByViews(bluePrints, "top", "bottom");


// Example of resulting data
// result looks like    
[
    {colorRef: {color: "red"}, views: ["front", "back", "top", "bottom"]},
    {colorRef: {color: "black"}, views: ["front", "back", "top", "bottom"]},
    {colorRef: {color: "silver"},  views: ["front", "back", "top", "bottom"]},
];
\$\endgroup\$
1
\$\begingroup\$

The initial data is not formatted as proper JSON. There appear to be mismatched quotes around values like front, back, etc... try running the snippet below to see the error that is caused by this.

const blueprints = {
  12: {color: red, views: [{name: "front}, {name: "back}, {name: "top}, {name: "bottom}]},
  231: {color: white, views: [{name: "front}, {name: "back}]},
  73: {color: black, views: [{name: "front}, {name: "back}, {name: "top}, {name: "bottom}]},
  4: {color: silver, views: [{name: "front}, {name: "back}, {name: "top}, {name: "bottom}]},
}
 

Also, unless red, white, black and silver are defined as variables, they need to be enclosed in quotes as string literals. See the rewrite below for a sample of this.


The indentation is inconsistent- the first level in the .map() callback is four spaces and then two spaces within the if block. Stick with one or the other.


You could use Array.prototype.reduce() to conditionally add the desired elements to an output array, reducing the need to call .filter():

const result = idArray.reduce((output,id) => {
  const bluePrint = bluePrints[id];
  //conditionally add to output and then return it
  return output;
}, []);

You could also use Object.keys() to iterate over the keys of bluePrints instead of idArray, though it appears to be slightly slower (see performance link below).

const result = Object.keys(bluePrints).reduce((output,id) => {
  const bluePrint = bluePrints[id];
  //conditionally add to output and then return it
  return output;
}, []);

See a comparison of the three snippets in this jsPerf.


One other change for readability would be to move that filter function outside the call to .map() or .reduce():

const viewIsTopOrBottom = view => view.name === 'top' || view.name === 'bottom'

Then that function can be called within the call to .some():

const exists = bluePrint.views.some(viewIsTopOrBottom);

And then that boolean exists could be eliminated by moving the call to .some() into the conditional statement of the if:

if (bluePrint.views.some(viewIsTopOrBottom)) {
  //add to array

Proposed Rewrite

const idArray = ["12", "231", "73", "4"];
  const bluePrints = {
    12: {color: "red", views: [{name: "front"}, {name: "back"}, {name: "top"}, {name: "bottom"}]},
    231: {color: "white", views: [{name: "front"}, {name: "back"}]},
    73: {color: "black", views: [{name: "front"}, {name: "back"}, {name: "top"}, {name: "bottom"}]},
    4: {color: "silver", views: [{name: "front"}, {name: "back"}, {name: "top"}, {name: "bottom"}]},
  };
const viewIsTopOrBottom = view => view.name === 'top' || view.name === 'bottom';
const result = idArray.reduce((output,id) => {
  const bluePrint = bluePrints[id];
  if (bluePrint.views.some(viewIsTopOrBottom)) {
    output.push({
      colorId: id,
      views: bluePrint.views
    });
  }
  return output;
}, []);
console.log('result,', result);

\$\endgroup\$
2
  • \$\begingroup\$ Nice answer, I +1'd. I still would have liked to see a final version with your final suggestions (which are all good) \$\endgroup\$ – konijn Nov 16 '18 at 9:26
  • \$\begingroup\$ Thanks- I restructured it; since the OP is unregistered I wonder if he or she will see it but maybe he or she will register (and as you know can then have accounts merged). \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Nov 17 '18 at 15:27
0
\$\begingroup\$

First, compose a valid JavaScript object or JSON

const blueprints = {
  "4":{"color":"silver","views":[{"name":"front"},{"name":"back"},{"name":"top"},{"name":"bottom"}]},
  "12":{"color":"red","views":[{"name":"front"},{"name":"back"},{"name":"top"},{"name":"bottom"}]},
  "73":{"color":"black","views":[{"name":"front"},{"name":"back"},{"name":"top"},{"name":"bottom"}]},
  "231":{"color":"white","views":[{"name":"front"},{"name":"back"}]}
}

The code can be shortened to a single line using Object.entries(), .map(), .every(), .find(), .concat() and spread syntax

const allProps=(a,i,p,A,B,R)=>[].concat(...Object.entries(a).map(([key,{[A]:k}])=>i.find(n=>n===key)&&p.every(n=>k.find(({[B]:j})=>n==j))?{[R]:key,[B]:k}:[]))

// `a`: input object, `ids`: `idArray`, `propNameA`: `"views"`
// `propNameB`: `"names"`, `retPropName`: `"colorId"`
const allProps = (a, ids, props, propNameA, propNameB, retPropName) =>
  [].concat(...Object.entries(a)
    .map(([key, {[propNameA]: k}]) => // destructure `propNameA`: `k
      // check if `key` is found in `ids`
      ids.find(n => n === key) 
      // and every `propNameB`: `p` is found 
      // in `props`: `["front", "back", "top", "bottom"]`
      && props.every(n => k.find(({[propNameB]: p}) => n === p))
      // if `true` return object having `retPropName` set to `key`
      // and `propNameA` set to `k`
      ? {[retPropName]: key, [propNameA]:k}
      // else return empty array, that is spread into new array
      : []
    )
  );

const idArray = ["12", "231", "73", "4"];

let names = ["front", "back", "top", "bottom"];

const blueprints = {
  "4":{"color":"silver","views":[{"name":"front"},{"name":"back"},{"name":"top"},{"name":"bottom"}]},
  "12":{"color":"red","views":[{"name":"front"},{"name":"back"},{"name":"top"},{"name":"bottom"}]},
  "73":{"color":"black","views":[{"name":"front"},{"name":"back"},{"name":"top"},{"name":"bottom"}]},
  "231":{"color":"white","views":[{"name":"front"},{"name":"back"}]}
}

let res = allProps(blueprints, idArray, names, "views", "name", "colorId");

console.log(res);

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.