0
\$\begingroup\$

I am trying to optimize my reducer code, preferably into one-liner with high-order-functions.

Reducer should return new array with names of groups, with new number.

Below are correct inputs and outputs:

Input:        |          Output:
              |
[]            |          ['A1'] 
['A1']        |          ['A1', 'A2']
['A1', 'A3']  |          ['A1', 'A2', 'A3']
['WORD', 'A2']|          ['WORD', 'A2', 'A1']

Code below is part of switch-case block:

case DrivesPageActions.DrivesPageActionTypes.AddCustomGroup:
  if (state.customGroups.includes('A1') === false) {
    return {
      ...state,
      customGroups: [...state.customGroups, 'A1']
    };
  } else if (state.customGroups.includes('A2') === false) {
    return {
      ...state,
      customGroups: [...state.customGroups, 'A2']
    };
  } else {
    return {
      ...state,
      customGroups: [...state.customGroups, 'A3']
    };
  }
\$\endgroup\$
  • \$\begingroup\$ A little context as to what this logic is used for would be great. Also, it looks like A2 in the third example does not follow the logic. It's inserted in between, not appended at the end. \$\endgroup\$ – Joseph Feb 1 at 16:04
1
\$\begingroup\$

preferably into one-liner with high-order-functions.

Machines execute code the same way, minified or not. But a human doesn't read or understand minified code the same way as verbose JS. Write for a human, don't one-line code.

Now one thing about your code. You do NOT compare booleans. if statements evaluate booleans. If your expression is already a boolean, then just use that boolean. If you need to negate it, use !.

case DrivesPageActions.DrivesPageActionTypes.AddCustomGroup:
  if (!state.customGroups.includes('A1')) {
    return {
      ...state,
      customGroups: [...state.customGroups, 'A1']
    };
  } else if (!state.customGroups.includes('A2')) {
    return {
      ...state,
      customGroups: [...state.customGroups, 'A2']
    };
  } else {
    return {
      ...state,
      customGroups: [...state.customGroups, 'A3']
    };
  }

Looks like your code is looking for A1, A2 and A3 in that order and adding the first one that's not found. You could just create an array of the three, then filter it by the array in the state, removing the ones already present. The first item in the filtered array is the item you want.

const inserts = ['A1', 'A2', 'A3']
const defaultInsert = inserts[inserts.length - 1]

function reducer(state, action){
  switch (action) {
    case 'AddCustomGroup':
      const missing = inserts.filter(v => !state.customGroups.includes(v))
      const insert = missing[0] || defaultInsert
      return { ...state, customGroups: [...state.customGroups, insert] }
    default:
      return state
  }
}

console.log(reducer({ customGroups: [] }, 'AddCustomGroup'))
console.log(reducer({ customGroups: ['A1'] }, 'AddCustomGroup'))
console.log(reducer({ customGroups: ['A1', 'A3'] }, 'AddCustomGroup'))
console.log(reducer({ customGroups: ['WORD', 'A2'] }, 'AddCustomGroup'))

\$\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.