1
\$\begingroup\$

I have the following code, that groups similar todos into an array of arrays, using lodash (https://lodash.com). Here is a plnkr http://plnkr.co/edit/t4J5bKGjMlQxW2z9Zh7x?p=preview (open your console and look into script.js)

function Todo(value) { this.value = value; this.isSimilar = function(todo) { return this.value == todo.value; }; } var _data = [ new Todo("hello"), new Todo("world"), new Todo("foo"), new Todo("bar"), new Todo("hello") ]; var processedTodos = []; var groups = _.compact(_.map(_data, function(todo) { if(processedTodos.indexOf(todo)>-1) { return; } var similarTodos = _.filter(_data, function(todo2) { return todo!==todo2 && todo.isSimilar(todo2); }); similarTodos.unshift(todo); // prepend todo to front of array [].push.apply(processedTodos, similarTodos); return similarTodos; })); console.log(groups); 

groups contains an array of arrays, where similar todos are in the same array.

For example, when I input this (these are JS objects, but for the sake of readability in JSON):

[ {"value":"hello"}, {"value":"world"}, {"value":"foo"}, {"value":"bar"}, {"value":"hello"} ] 

It returns this:

[ [{"value":"hello"},{"value":"hello"}], [{"value":"world"}], [{"value":"foo"}], [{"value":"bar"}] ] 

It groups together similar objects.

But the whole code doesn't feel right, there must be a better way to do this!

Another bad thing is that I need an additional array to store the already checked todos (which avoid me duplicate arrays like [todo1,todo2] and [todo2,todo1].

Last but not least, I have to remove undefined values from the array with _.compact, because this line returns undefined when the todo was already processed:

 if(processedTodos.indexOf(todo)>-1) { return; } 

Any idea how to improve this code?

\$\endgroup\$
2
  • \$\begingroup\$Can you add a sample input and its corresponding output?\$\endgroup\$
    – Joseph
    CommentedNov 24, 2015 at 14:28
  • \$\begingroup\$I've added some input and output, and there is also a plnkr link, if you missed it: plnkr.co/edit/t4J5bKGjMlQxW2z9Zh7x?p=preview\$\endgroup\$
    – 23tux
    CommentedNov 24, 2015 at 14:45

1 Answer 1

3
\$\begingroup\$

First, using a constructor is overkill. Your Todos don't use inheritance nor share methods (the method is created per instance, thus not shared). You can simply have a factory (a function that returns an object). isSimilar could be just a helper function that has nothing to do with Todo.

function createTodo(value){ return { value: value, }; } var _data = [ createTodo("hello"), createTodo("world"), createTodo("foo"), createTodo("bar"), createTodo("hello") ]; 

So now, you have a list of objects. You can then use reduce and build up a hash of objects with the value as their key, and an array as the value.

var todosByValue = _data.reduce(function(hash, todo){ if(!hash.hasOwnProperty(todo.value)) hash[todo.value] = []; hash[todo.value].push(todo); return hash; }, {}); // now we have something like: { hello: [{ value: 'hello' }, { value: 'hello' }], world: [{ value: 'world' }], ... } 

All you need to do is just pull the values out to an array using Object.keys and array.map.

var groupedByTodo = Object.keys(todosByValue).map(function(key){ return todosByValue[key]; }); 

In all, it should simply look like this:

function createTodo(value){ return { value: value, }; } var _data = [ createTodo("hello"), createTodo("world"), createTodo("foo"), createTodo("bar"), createTodo("hello") ]; var todosByValue = _data.reduce(function(hash, todo){ if(!hash.hasOwnProperty(todo.value)) hash[todo.value] = []; hash[todo.value].push(todo); return hash; }, {}); var groupedByTodo = Object.keys(todosByValue).map(function(key){ return todosByValue[key]; }); 
\$\endgroup\$

    Start asking to get answers

    Find the answer to your question by asking.

    Ask question

    Explore related questions

    See similar questions with these tags.