2
\$\begingroup\$

Context: I am making a website that allows people to upload their Facebook Messenger data, and see data visualizations for conversations. It is a React front-end, and a nodejs back-end. It doesn't store anything in a database at the moment, but there is a caching layer to deal with different sessions. So once the user uploads their facebook messenger data, the nodejs back-end is supposed to crunch the numbers and aggregate different statistics.

The json looks like:

{
  "participants": [{"name":"Adam"}, {"name":"Bob"}, {"name":"Chris"}],
  "messages": [
    {
      "sender_name": "Adam",
      "timestamp_ms": 1533332999910,
      "content": "I am editing my stackexchange question right now",
      "type": "Generic"
    },
    {
      "sender_name": "Bob",
      "timestamp_ms": 1533332999910,
      "content": "Okay, good luck",
      "type": "Generic"
    }
  ],
  "title": "High School Friends",
  "is_still_participant": true,
  "thread_type": "RegularGroup",
  "thread_path": "inbox/highschoolfriends__aAe4vb"
}

messages is an array of objects, with common text messages having the shape of the above, with other fields that are optional like photos, audio_files, etc.

To compute the set up the data I use:

let jsonMessages = require('../analytics/message_1.json');

let participants = jsonMessages.participants;
let messages = jsonMessages.messages;

let people =  participants.map(function(person) {
    return person.name;
});

So now the data is a javascript object.

So my question: Am I missing something obvious about the map/filter/reduce powers by doing a big for loop of filters? Would this pass code review for an internal analytics tool?

Here is what I have tried:

let people = ['Adam', 'Bob', 'Chris', 'Dave'];

people.forEach(function (person) {
    let obj = {};
    obj.name = person;
    obj.messageCount = messages.filter((obj) => obj.sender_name === person).length;

    obj.photoMessageCount = messages.filter((obj) => ((obj.sender_name === person) && ((obj.photos != null) )).length;

    obj.videoGifMessageCount = messages.filter((obj) => ((obj.sender_name === person) && ((obj.gifs !=null) || (obj.videos!= null))).length;

    obj.audioMessageCount = messages.filter((obj) => ((obj.sender_name === person) && (obj.audio_files != null))).length;

    obj.richContentMessageCount = obj.photoMessageCount + obj.videoGifMessageCount + obj.audioMessageCount;

    stats.push(obj);
});
\$\endgroup\$
0

1 Answer 1

4
\$\begingroup\$

A big inefficiency of the initial approach is that for each person (which is actually a person's name) messages collection will be traversed 4 times.

To significantly optimize the traversal algorithm the current one is substituted with a single loop with multiple conditions per iteration.
All the needed specific counts are declared beforehand and used as accumulators.
The Consolidate conditional expression technique is applied based on a common condition m.sender_name === personName.

The final optimized version:

people.forEach((personName) => {
    let stat = {'name': personName, 'messageCount': 0, 'photoMessageCount': 0, 
               'videoGifMessageCount': 0, 'audioMessageCount': 0};
    messages.forEach((m) => {
        if (m.sender_name === personName) { 
            stat.messageCount++;
            if (m.photos) stat.photoMessageCount++;
            if (m.gifs || m.videos) stat.videoGifMessageCount++;
            if (m.audio_files) stat.audioMessageCount++;
        }
    });
    stat.richContentMessageCount = stat.photoMessageCount + stat.videoGifMessageCount 
                                   + stat.audioMessageCount;
    stats.push(stat);
});

In case if your stats array is intended to be used only for accumulating statistics from people.forEach(...) iterated once - prefer Array.map approach to create stats array with the results at once:

let stats = people.map((personName) => {
    let stat = {'name': personName, 'messageCount': 0, 'photoMessageCount': 0, 
               'videoGifMessageCount': 0, 'audioMessageCount': 0};
    // all the logic here
    // ...
    return stat;
});
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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