1
\$\begingroup\$

Data Structure coming back from the server

[
  {
    id: 1,
    type: "Pickup",
    items: [
      { 
        id: 1, 
        description: "Item 1"
      }
    ]
  },
  {
    id: 2,
    type: "Drop",
    items: [
      { 
        id: 0, 
        description: "Item 0"
      }
    ]
  },
  {
    id: 3,
    type: "Drop",
    items: [
      { 
        id: 1, 
        description: "Item 1"
      },
      { 
        id: 2, 
        description: "Item 2"
      }
    ]
  },
  {
    id: 0,
    type: "Pickup",
    items: [
      { 
        id: 0, 
        description: "Item 0"
      },
      { 
        id: 2, 
        description: "Item 2"
      }
    ]
  }
];
  • Each element represents an event.
  • Each event is only a pickup or drop.
  • Each event can have one or more items.

Initial State

On initial load, loop over the response coming from the server and add an extra property called isSelected to each event, each item, and set it as false as default. -- Done.

This isSelected property is for UI purpose only and tells user(s) which event(s) and/or item(s) has/have been selected.

// shove the response coming from the server here and add extra property called isSelected and set it to default value (false)
const initialState = {
  events: []
}

moveEvent method:

const moveEvent = ({ events }, selectedEventId) => {
  // de-dupe selected items 
  const selectedItemIds = {};
  
  // grab and find the selected event by id
  let foundSelectedEvent = events.find(event => event.id === selectedEventId);
  
  // update the found event and all its items' isSelected property to true
  foundSelectedEvent = {
    ...foundSelectedEvent, 
    isSelected: true,
    items: foundSelectedEvent.items.map(item => {
      item = { ...item, isSelected: true };
      // Keep track of the selected items to update the other events.
      selectedItemIds[item.id] = item.id;
      return item;
    }) 
  };

  events = events.map(event => {
    // update events array to have the found selected event
    if(event.id === foundSelectedEvent.id) {
      return foundSelectedEvent;
    }
    
    // Loop over the rest of the non selected events
    event.items = event.items.map(item => {
      // if the same item exists in the selected event's items, then set item's isSelected to true.
      const foundItem = selectedItemIds[item.id];
      // foundItem is the id of an item, so 0 is valid
      if(foundItem >= 0) {
        return { ...item, isSelected: true };
      }
      return item;  
    });
    
    const itemCount = event.items.length;
    const selectedItemCount = event.items.filter(item => item.isSelected).length;
    
    // If all items in the event are set to isSelected true, then mark the event to isSelected true as well.
    if(itemCount === selectedItemCount) {
      event = { ...event, isSelected: true };
    }

    return event;
  });

  return { events }
}

Personally, I don't like the way I've implemented the moveEvent method, and it seems like an imperative approach even though I'm using find, filter, and map. All this moveEvent method is doing is flipping the isSelected flag.

  1. Is there a better solution?
  2. Is there a way to reduce the amount of looping? Maybe events should be an object and even its items. At least, the lookup would be fast for finding an event, and I don't have to use Array.find initially. However, I still have to either loop over each other non selected events' properties or convert them back and forth using Object.entries and/or Object.values.
  3. Is there more a functional approach? Can recursion resolve this?

Usage and Result

// found the event with id 0
const newState = moveEvent(initialState, 0);

// Expected results
[
  {
    id: 1,
    type: 'Pickup',
    isSelected: false,
    items: [ { id: 1, isSelected: false, description: 'Item 1' } ]
  }
  {
    id: 2,
    type: 'Drop',
    // becasue all items' isSelected properties are set to true (even though it is just one), then set this event's isSelected to true
    isSelected: true,
    // set this to true because event id 0 has the same item (id 1)
    items: [ { id: 0, isSelected: true, description: 'Item 0' } ]
  }
  {
    id: 3,
    type: 'Drop',
    // since all items' isSelected properties are not set to true, then this should remain false.
    isSelected: false,
    items: [
      { id: 1, isSelected: false, description: 'Item 1' },
      // set this to true because event id 0 has the same item (id 2)
      { id: 2, isSelected: true, description: 'Item 2' }
   ]
  }
  {
    id: 0,
    type: 'Pickup',
    // set isSelected to true because the selected event id is 0
    isSelected: true,
    items: [
      // since this belongs to the selected event id of 0, then set all items' isSelected to true
      { id: 0, isSelected: true, description: 'Item 0' },
      { id: 2, isSelected: true, description: 'Item 2' }
    ]
  }
]
\$\endgroup\$
5
  • \$\begingroup\$ Please describe what moveEvent does. \$\endgroup\$ – Blindman67 Nov 18 '20 at 1:15
  • \$\begingroup\$ I thought my post was clear and detailed. Anyways, moveEvent is just a method that flips isSelected from false to true based on the given event id (2nd parameter). When given an event id (AKA selected event), it will find the event, set its' isSelected to true, loop over each item belonging to that event, and set each item's isSelected to true. \$\endgroup\$ – FNMT8L9IN82 Nov 18 '20 at 2:07
  • \$\begingroup\$ Then, loop over the rest of the other events (AKA non selected events) and items, if the item in non selected event is the same item in the selected event (by id), then mark the item's isSelected to true. If all items' isSelected are set to true in a non selected event, then set the non selected event's isSelected to true too. \$\endgroup\$ – FNMT8L9IN82 Nov 18 '20 at 2:07
  • \$\begingroup\$ The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$ – BCdotWEB Nov 18 '20 at 9:49
  • \$\begingroup\$ Would it be helpful to simplify the original post and basically say I want this array transform into this array? \$\endgroup\$ – FNMT8L9IN82 Nov 18 '20 at 10:11
2
\$\begingroup\$

You can just get all selected items and compute everything in one go using functional approach with map:

const moveEvent = ({ events }, selectedEventId) => {
    const selectedEvent = events.find(({id}) => id === selectedEventId);
    const selectedItems = new Set(selectedEvent.items.map(({id}) => id));
    const allItemsSelected = (items) => items.every(({ id }) => selectedItems.has(id));

    return {
        events: events.map((event) => ({
            ...event,
            isSelected: (selectedEventId === event.id || allItemsSelected(event.items)),
            items: event.items.map((item) => ({
                ...item,
                isSelected: selectedItems.has(item.id)
            }))
        }))
    };
};

If you want to be truly functional you can transform temp variables at the beginning of the moveEvent to pure functions.

\$\endgroup\$
7
  • \$\begingroup\$ Thank you so much! I like this approach over stackoverflow. This is easier and less to maintain. \$\endgroup\$ – FNMT8L9IN82 Nov 19 '20 at 0:12
  • \$\begingroup\$ Dumb question...isn't this already functional? it's a pure function that takes in two arguments, no global variables, and returns new data each time without modifying anything. I'm not sure what you mean about transform temp variables at the beginning. Isn't it already defining variables at the beginning? \$\endgroup\$ – FNMT8L9IN82 Nov 19 '20 at 0:15
  • \$\begingroup\$ @FNMT8L9IN82 I think, but I'm not 100% sure, that the idea is that functional programming avoids mutating scope - that is, variables are never defined except in arguments, resulting in functions that take arguments and immediately return new values every time. \$\endgroup\$ – CertainPerformance Nov 19 '20 at 3:54
  • \$\begingroup\$ @FNMT8L9IN82 yes, for the external observer it is pure function. But internally moveEvent has a sequence of statements and that is forbidden in purely functional languages. \$\endgroup\$ – Aleksei Tirman Nov 19 '20 at 6:42
  • \$\begingroup\$ So you mean that I should take out selectedEvent, selectedItems, allItemsSelected into their own functions, and have moveEvent call those functions? \$\endgroup\$ – FNMT8L9IN82 Nov 19 '20 at 12:32

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.