Destructuring an object in JS

Question

The solution I have written is working as expected, but here is my function:

const dataExtraction = () => {
 const { id, name, sprites, types } = data;
 const pokemonType= types[0].type.name;
 const { front_default } = sprites;
 const animated= sprites.versions['generation-v']['black-white'].animated.front_default;
 
 return { 
 id, //6
 name, //charizard
 pokemonType, //fire
 front_default, //image url
 aniamted //image url
 }
}

I have the JSON payload below and I have extracted data that I would like to use. I have followed a pretty standard way to navigate into the nested objects but I would like to get people opinion if there is a better solution or cleaner way of writing above function:

https://pastebin.com/7LUPLbMm

• konijn: aniamted -> animated ?

Answer 1

1. Using an arrow function as inline function does not make the code very readable.
2. Why assign variables if they are only used once?

function extractData (json) { 
 return { 
 id: json.id,
 name: json.name,
 pokemonType: json.types[0].type.name,
 front_default: json.sprites.front_default,
 animated: json.sprites.versions['generation-v']['black-white'].animated.front_default
 };
}

Answer 2

A super short review;

• data seems to be a global variable with a super generic name, that's a bad idea
• Destructuring is cool, but personally, this feels like too much
• function names are often <verb><subject> so perhaps extractData, or even extractPokemon?
• I like your use of const and comments

I was going to write a counter example, but Richard N took care of that.

Answer 3

I think the two previous answers offer improvements, but I'd like to throw another suggestion into the mix :)

When dealing with JSON like this, the fragility from the deep lookups send shivers down my spine - it looks error prone to me, because if the data or data shape changes, e.g. has undefined or null somewhere in the chain, this code is going to break.

Rather than suggest a try/catch approach, which could work, I'd recommend that you use a well tested library method such as get from lodash.

You'll then have something a bit more robust, like so:

import { get } from 'lodash';
const mapJsonToPokemonItem = (data) => ({
 id: get(data, 'id'),
 name: get(data, 'name'),
 pokemonType: get(data, 'types[0].type.name'),
 front_default: get(data, 'sprites.front_default'),
 animated: get(data, "sprites.versions['generation-v']['black-white'].animated.front_default"),
});

Note the following:

1. I've named the function mapJsonToPokemonItem in an attempt to better describe what the function actually does.
2. Accept the data as an argument, to avoid reliance on the outside scope for this variable.
3. Directly return an object with the => ({}) syntax, as there's no real need for execution within the function, other than the mapping.
4. You can optionally provide a default value for the get calls, e.g.: get(object, 'a.b.c', 'default'), which may be useful for fallback images, etc.

• Bergi: "if the data or data shape changes, this code is going to break" - I would think that's expected. I would want my code to cause an exception if the data has a different shape than anticipated, instead of silently coercing it to undefined.
• pjlangley: @Bergi How you handle the changed/missing data shape is mostly subjective. I've put forward something graceful, if viewing partial data is acceptable, but I understand your POV. I guess we'd need input from the OP on what they'd prefer to do, and what makes sense for their users. Thanks for the edit btw :)
• RoToRa: If you are using a modern runtime environment (or a JS compiler such as Babel) then instead of lodash's get you should also consider optional chaining ?.. It has the advantage not to "hide" the properties in a string: data?.sprites?.versions?.['generation-v']?.['black-white']?.animated?.front_default
• Robert Koritnik: Since when are magic strings considered robust? If at all I try to avoid all magic values in my code because they most of the time typos in them can't be caught during compile time.