2

I'm working with ES6 and React. I'm parsing through a response object from a Java Rest service, and it dawned on me that there's probably a cleaner way of doing parsing an object into two objects. This wokrs, it just looks clunky.

            let draftList = [];
            let readyForApprovalList = [];
            for (let i = 0; i < action.allStatusesSurveysList.length; i++){
                if (action.allStatusesSurveysList[i].status === statusTypes.DRAFT){
                    draftList.push(action.allStatusesSurveysList[i]);
                } else if (action.allStatusesSurveysList[i].status === statusTypes.READY_FOR_APPROVAL){
                    readyForApprovalList.push(action.allStatusesSurveysList[i]);
                }
            }
nikotromus
  • 1,015
  • 16
  • 36

2 Answers2

2

A combination of Array.prototype.filter() and arrow filter functions would be a pretty clean solution.

Note that while this solution is nice and readable, it is not the most efficient one as it takes at least two iterations of the array. However, it may very well be efficient enough for the vast majority of use cases and for this reason a good trade-off.

const draftList = action.allStatusesSurveysList 
    .filter(i => i.status === statusTypes.DRAFT)
const readyForApprovalList = action.allStatusesSurveysList
    .filter(i => i.status === statusTypes.READY_FOR_APPROVAL)

You could even extract the status filter function further:

const byStatus = status => item => item.status === status

const draftList = action.allStatusesSurveysList
    .filter(byStatus(statusTypes.DRAFT))
const readyForApprovalList = action.allStatusesSurveysList
    .filter(byStatus(statusTypes.READY_FOR_APPROVAL))
TimoStaudinger
  • 41,396
  • 16
  • 88
  • 94
1

forEach would probably work better here - while you can use filter, it would require one pass for each category type.

action.allStatusesSurveysList.forEach(function(item) {
    switch(item.status) {
        case statusTypes.DRAFT:
            draftList.push(item);
            break;
        case statusTypes.READY_FOR_APPROVAL:
            readyForApprovalList.push(item);
            break;
        default:
            // "unknown status" list??
    }
});

This will iterate over the array and has the neat extra of using item instead of repeatedly putting action.allStatusesSurveysList[i]. Also, using a switch statement is better than if..else if..

Niet the Dark Absol
  • 320,036
  • 81
  • 464
  • 592
  • I think splitting the loop and running two `filter`s is a more optimised approach than `forEach` and `switch-case`s, since there are only two cases. That `switch-case` will internally convert to a `if-else` anyway. – Quirk Dec 07 '16 at 15:59
  • @Quirk Possibly, but the `filter` approach iterates over the same array multiple times, and that feels clunky to me when only one iteration is needed. I also feel that my `switch` method is more expandable if future statuses are needed. – Niet the Dark Absol Dec 07 '16 at 16:00
  • I agree when there are more than two cases involved, it makes sense and is more readable. Otherwise, I'm just about always thinking about this: http://stackoverflow.com/a/11227902/2844164 – Quirk Dec 07 '16 at 16:03
  • You are very hard to tag! :P – Quirk Dec 07 '16 at 16:04