Является ли такой код плохим?

Question

[Vadim Stepanenko]

привет!
имеется такой код:

const SECTIONS = [
    'top',
    'bottom',
    'head',
    'shoes',
    'bag',
    'accs',
];

const prepareShareItems = () => {
    const person = store.getState().globals.person; // объект { top: { slide: 0, list: [...] }, bottom: { slide: 0, list: [...] }, head: { slide: 0, list: [...] } etc}
    const items = store.getState().globals.items; // массив объектов

    let result;
    for (let i = 0; i < SECTIONS.length; i++) {
        const _section = SECTIONS[i];
        const _itemIndex = person[_section].slide;
        const _itemId = person[_section].list[_itemIndex];

        const _item = items.find((item) => item.id === _itemId);
        result = {
            ...result,
            [_section]: _item.image,
        };
    }

    return result;
};

в двух словах про алгоритм: получаем массив секций (SECTIONS), далее каждый элемент массива используем в качестве ключа person; получаем slide каждой секции в person; item id у которого индекс _itemIndex; ищем в items объект с ид _itemIndex и забируем у него image и закидываем в result.

Вопрос следующий: является ли это плохим кодом? Да, многие вещи можно сократить, но так уж с университета повелось, что каждый шаг алгоритма мне для понимания удобнее писать отдельно. Просто часто вижу что подобные алгоритмы пишут чуть ли не в одну длинную строку, где ничего не понятно и не структурировано

Comments

[WbICHA]

Вот это:

let result;
    for (let i = 0; i < SECTIONS.length; i++) {
        const _section = SECTIONS[i];
        const _itemIndex = person[_section].slide;
        const _itemId = person[_section].list[_itemIndex];

        const _item = items.find((item) => item.id === _itemId);
        result = {
            ...result,
            [_section]: _item.image,
        };
    }

Ужас-ужас. Для этой задачи есть редьюс. И названия переменных это отдельный вид извращения.

И тут непонятно зачем дважды запрашивал один и тот же стейт. Да и в целом деструтуризация здесь напрашивается.

const person = store.getState().globals.person; // объект { top: { slide: 0, list: [...] }, bottom: { slide: 0, list: [...] }, head: { slide: 0, list: [...] } etc}
    const items = store.getState().globals.items; // массив объектов

[WbICHA]

И, кстати, а зачем ты каждый раз массив пересоздаёшь?

result = {
            ...result,
            [_section]: _item.image,
        };

Пушить в существующий не учили?

[Vadim Stepanenko]

Пушить в существующий не учили?

Привычка от использования редакса :(

И названия переменных это отдельный вид извращения.

Ну вообще да, щас перегнул с _. Обычно использую _ перед переменной, если она содержит в себе какие-то промежуточные значения, необходимые для дальнейших этапов

И тут непонятно зачем дважды запрашивал один и тот же стейт. Да и в целом деструтуризация здесь напрашивается.

Очень могу ошибаться, но где-то читал что в таком случае сначала берется весь стейт, а потом деструктуризуется на части, что не оч для производительности (например если стейт очень большой, а нам нужно только 2 объекта из него). А в моем варианте мы сразу берем нужные части стейта

[WbICHA]

Vadim Stepanenko,

Очень могу ошибаться, но где-то читал что в таком случае сначала берется весь стейт, а потом деструктуризуется на части, что не оч для производительности (например если стейт очень большой, а нам нужно только 2 объекта из него). А в моем варианте мы сразу берем нужные части стейта

Возможно, я чего-то не знаю, но логики я здесь не вижу совсем.
Глобал ─ это объект, объект передаётся по ссылке.
Дестрктуризация ─ это синтаксический сахар.
Поэтому каким образом вот это:

const globals = store.getState().globals;
const person = globals.person;
const items = globals.items;

Может быть менее производительно чем это:

const person = store.getState().globals.person; // объект { top: { slide: 0, list: [...] }, bottom: { slide: 0, list: [...] }, head: { slide: 0, list: [...] } etc}
    const items = store.getState().globals.items; // массив объектов

Я не знаю.
А вот наоборот ─ да.

[WbICHA]

Vadim Stepanenko, совсем забыл:

Обычно использую _ перед переменной, если она содержит в себе какие-то промежуточные значения, необходимые для дальнейших этапов

В жсе _ используется в начале переменных, которые НЕ используются. Используются в функциях, в которых нам необходимы аргументы после какого-то.
const f = (_a, b, _c, d, e) => { ... };
b, d и e будут использоваться, а и с ─ нет.

Answer 1

[WbICHA]

Современное оформление:

const SECTIONS = [
    'top',
    'bottom',
    'head',
    'shoes',
    'bag',
    'accs',
  ];

  const prepareShareItems = () => {
    const { person, items } = store.getState().globals;
    
    const result = SECTIONS.reduce((acc, section) => {
      const personSection = person[section];
      const { slide } = personSection;
      const itemId = personSection.list[slide];
      
      const item = items.find((item) => item.id === itemId);
      acc[section] = item?.image;
      
      return acc;
    }, {});
    
    return result;
  }

Answer 2

[TheOnlyFastCoder2]

Вроде по максимуму сократил и улучшил.
Если вы как-нибудь исправите(улучшите) названия своих ключей , код будет еще лучше смотреться

const SECTIONS = ['top','bottom','head','shoes','bag','accs',];

const prepareShareItems = ({person,items}) => {    
    for (var  i = 0 , res = {} ; i < SECTIONS.length; i++) {
      	const {slide,list}  = person[SECTIONS[i]];
      	Object.assign(res,{[SECTIONS[i]]: items.find((item) => item.id === list[slide]).image})
    }
    return res;
};

prepareShareItems(store.getState().globals)

Comments

[Vadim Stepanenko]

спасибо, так код меньше и все равно читается

[TheOnlyFastCoder2]

Vadim Stepanenko, Попробуй сделать так

const prepareShareItems = ({person,items}) => {    
    for (var  i = 0 , res = {} ; i < SECTIONS.length; i++) {
      	const {slide,list}  = person[SECTIONS[i]];
      	Object.assign(res,{[SECTIONS[i]]: items.find((item) => item.id === list[slide]).image})
    }
    return res;
};

И если я помог , то отметьте пожалуйста мой ответ за решенное

[Alex]

Кода стало меньше, но что его легче читать я бы не сказал (* ￣︿￣)

[WbICHA]

TheOnlyFastCoder2,

return result;

А откуда резалт?

[TheOnlyFastCoder2]

WbICHA, result поменял , на res . Забыл переименовать ((