0

I am doing a personal project in react.js but at this moment I am trying to write clean code. Could exist a way to write shorter this code in two lines? or something like that. I tried to make objects of arrays like

const apps= {app_name:['Maps','Meet'], position:['0 2323', '0 2323'}

but It does not work

const apps = [
  {
    app_name: 'Maps',
    position: '0 -828px'
  },
  {
    app_name: 'Gmail',
    position: '0 -483px'
  },
  {
    app_name: 'Calendar',
    position: '0 -2553px'
  },
  {
    app_name: 'Drive',
    position: '0 -2277px'
  }
];

There is not a way to rewrite it in a more shorter way in order to make the map's function works?

{apps.map((el, i) =>
    <li className='items'>
        <a className='link'>
            <div className='img_box'>
                <span className='img'
                  style={ { backgroundPosition: el.position } }></span>
            </div>
            <span className='app_name'>{el.app_name}</span>
        </a>  
    </li>    
)}

terrymorse
  • 6,771
  • 1
  • 21
  • 27
  • Since every app_name value is different you wont get much cleaner then json, if it was only position id suggest a loop, but might be something you regret later if one item need tweaking. Single line per item would be shorter/cleaner (4 lines vs 16), but your linter/formatter may complain – Lawrence Cherone Jun 01 '20 at 14:57

3 Answers3

2

This might be more compact, it comes down to personal preference.

You could create two arrays:

const appNames = ['Maps', 'Gmail', 'Calendar', 'Drive'];
const positions = ['0 -828px', '0 -483px', '0 -2553px', '0 -2277px'];

Then use the arrays to form your final array:

let apps = appNames.map((el, i) => {
  return { app_name: el, position: positions[i] };
});

const appNames = ['Maps', 'Gmail', 'Calendar', 'Drive'];
const positions = ['0 -828px', '0 -483px', '0 -2553px', '0 -2277px'];

let apps = appNames.map((el, i) => {
  return { app_name: el, position: positions[i] };
});

document.getElementById('result').innerHTML = 
  'apps:' + JSON.stringify(apps, null, 2);
<pre id="result"></pre>
terrymorse
  • 6,771
  • 1
  • 21
  • 27
1

An idea would be to write a function that creates the app structure:

function create_app(app_name, position) {
    return ({ app_name, position });
}

const apps = [
    create_app('Maps', '0 -828px'),
    create_app('Gmail', '0 -483px'),
    create_app('Calendar', '0 -2553px'),
    create_app('Drive', '0 -2277px')
];

Another idea would be to define an object with the app name as key and position as value, but that might be a problem in the long run if you decide later that you want to associate apps with anything other than positions.

const app_positions = {
    'Maps': '0 -828px',
    'Gmail': '0 -483px',
    'Calendar': '0 -2553px',
    'Drive': '0 -2277px'
};

{app_positions.keys().map((app_name) =>
    <li className='items'>
        <a className='link'>
            <div className='img_box'>
                <span className='img'
                  style={ { backgroundPosition: app_positions[app_name] } }></span>
            </div>
            <span className='app_name'>{app_name}</span>
        </a>  
    </li>    
)}
rid
  • 61,078
  • 31
  • 152
  • 193
  • Nice! Thanks! Could you explain me why the object of arrays don't work in this case? – Omar Cardenas Jun 01 '20 at 14:54
  • @OmarCardenas, [`Object`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object) does not define a `map()` function the way [`Array`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array) does, so `map` on `apps` if `apps` is an `Object` would be `undefined`. – rid Jun 01 '20 at 14:58
  • @OmarCardenas, added an alternative, but I think your original idea of an array with objects is the best for the future. – rid Jun 01 '20 at 15:03
  • Thanks! I get your point. I complained a little in the beginning because I thought my original idea was much larger than others and It could not be the right way to code properly – Omar Cardenas Jun 01 '20 at 15:07
  • @OmarCardenas, in general, good code has to be clear, easy to understand and non-repetitive, not necessarily short. – rid Jun 01 '20 at 15:08
  • Ohhh kk. It would not matter If I had to write 20 objects inside an array? It is clear, easy to understand but It is repetitive I think – Omar Cardenas Jun 01 '20 at 15:11
  • @OmarCardenas, true, but the nature of the task is repetitive, because this is data, not code. That's why people recommend loading a JSON file that contains the data. If you keep your original array of apps, and instead of writing the data in code you load it from a file, then your code will be reduced to one line that's clear and easy to read and understand: `const apps = JSON.parse(await fetch("/apps.json"));`. – rid Jun 01 '20 at 15:14
  • (or you could use a bundler to bundle the data inside the JavaScript code at compile time, instead of making a request at runtime) – rid Jun 01 '20 at 15:20
1

Well typically, your array should not be a part of your code. It should be a separate JSON file.

From your code I see that you want to display a list with the content of the array. So my point still holds. Data should not be part of code.

Even if you are writing a jest file, data ideally should be a separate file. You can then write a neater logic to get it from the JSON file or say a web API based on a configuration.

Good day to you.

SamwellTarly
  • 802
  • 8
  • 15