current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 0 down vote favorite

I have an array of API objects, below is an example of one object:

{
    description: "Get a list of most recent social alerts"
    example_input: ""
    example_output: "status"
    id: 19
    method: "GET"
    resource: "/api/alerts"
}

With my code below, I'm able to loop through the big Array, then filter out items into 1 of 4 arrays (for GET, POST, PUT and DELETE)

RestFactory.getREST().then(function(res) {
    filterArrays(res.data.rest_methods);
});

function filterArrays(data) {
    for (var i=0; i<data.length; i++) {
        if (data[i].method === 'GET') {
            getArray.push(data[i]);
        }
        else if (data[i].method === 'POST') {
            postArray.push(data[i]);
        }
        else if (data[i].method === 'PUT') {
            putArray.push(data[i]);
        }
        else if (data[i].method === 'DELETE') {
            deleteArray.push(data[i]);
        }
    }
}

I feel that this is not the most efficient way to accomplish this, how should this be refactored? Would you recommend lodash in this scenario?

share|improve this question
    
You are basically looking for groupBy, like groupBy(data, x => x.method) – elclanrs Mar 9 at 20:46
up vote 4 down vote accepted

This can easily be done with array.reduce

let resultsByMethod = data.reduce((results, result) => {
  if(!results[result.method]) results[result.method] = [];
  results[result.method].push(result);
  return results;
}, {});

The end result is something like:

{
  GET: [...],
  POST: [...],
  PUT: [...],
  DELETE: [...]
}

One disadvantage in your approach is when it encounters a method that's note one of the 4 you're comparing.

share|improve this answer
    
Thanks, this is an es6 example however – Leon Gaban Mar 9 at 21:38
up vote 0 down vote

Ended up with this, using _.each and a switch case:

function filterColumns(apis) {
    _.each(apis, function(api) {
        switch (api.method) {
            case 'GET'    : getArray.push(api);    break;
            case 'POST'   : postArray.push(api);   break;
            case 'PUT'    : putArray.push(api);    break;
            case 'DELETE' : deleteArray.push(api); break;
        }
    });
}

With the original for loop, each item had the possibility of being checked a maximum of 4 times.

Using _.each simplified the loop, making it much more readable.

Next using a switch case solves the problem of if statements having to be checked for multiple cases. The correct case will pass immediately using the switch case.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.