1
\$\begingroup\$

Because it is a lot of code, I don't want to ask for details what is wrong, but want to ask about review what can be changed in approach to this.

This is controller that make query:

  • It grabs data within time range
  • Does aggregation so I get sum of some fields
  • Because there are some fields from different collections i.e leader_name, I do other queries
  • There is a $in: workers fragment in the $match part that also need workers to filter from a different query

getGroupInRange: function (req, gres) {

    console.log(req.params);

    Group.find({name: {$in: req.params.id.split(',')}})
      .populate('workers')
      .exec(function(err, found){

        if (err) return gres.serverError(err);

        var workers = [];

        found.forEach(function(x){
          x.workers.forEach(function(xx){
            workers.push(xx.id);
          });
        });

        workers = workers.map(function (x) {
          return new ObjectId(x);
        });



        Event.native(function(err, collection) {
          if (err) return gres.serverError(err);

          var query = [
            {
              $match:
              {
                probe_time:
                {
                  $gte: new Date(req.params.from),
                  $lt: new Date(req.params.to)
                },
                worker_id:
                {
                  $in: workers
                }
              }
            },
            {
              $group:
              {
                _id:{worker_id:"$worker_id", group:"$group"},
                "total_logged_time":{$sum: "$duration"},
                "total_break_time":{$sum: {$cond: [ {$eq:["$user.work_mode", 'break']},"$duration", 0] }},
                "total_nonidle_duration":{$sum: {$cond: [ {$eq:["$user.presence", 'active']},"$duration", 0] }},
                "total_idle_time":{$sum: {$cond: [ {$eq:["$user.presence", 'idle']},"$duration", 0] }},
                "total_pro_apps_time":{$sum: {$cond: [ {$eq:["$app_category", 'productive']},"$duration", 0] }},
                "total_nonpro_apps_time":{$sum: {$cond: [ {$ne:["$app_category", 'productive']},"$duration", 0] }},
                "status1":{$sum: {$cond: [ {$eq:["$user.work_mode", 'CUSTOM_1']},"$duration", 0] }},
                "status2":{$sum: {$cond: [ {$eq:["$user.work_mode", 'CUSTOM_2']},"$duration", 0] }},
                "status3":{$sum: {$cond: [ {$eq:["$user.work_mode", 'CUSTOM_3']},"$duration", 0] }},
                "status4":{$sum: {$cond: [ {$eq:["$user.work_mode", 'CUSTOM_4']},"$duration", 0] }},

                "leader_name":{$last:"$leader_name"},
              }
            },
            {
              $project:
              {
                "total_logged_time":1,
                "total_break_time":1,
                "total_nonidle_duration":1,
                "total_idle_time":1,
                "total_pro_apps_time": 1,
                "total_nonpro_apps_time" : 6388,
                "print_qty":1,
                "medium_print_qty":1,
                "medium_download_size":1,
                "effectivity":1,
                "status1":1,
                "status2":1,
                "medium_break_time": { $multiply:[3600, {$divide:["$total_break_time", "$total_logged_time"]} ] },
                "worker_name":"$_id.worker_id",
                "leader_name":1
              }
            },
            {
              $group:
              {
                _id:"$_id.group",
                data:{$push: "$$ROOT"}
              }
            }
          ];


          collection.aggregate(query).toArray(function (err, results) {
            if (err) return res.serverError(err);

            Worker.find({_id:{$in: workers}})
              .populate('leader')
              .exec(function (err, res) {

                results.map(function(x,y){
                  x.data.map(function (xx, yy) {

                    xx.leader_name = res.filter(function (x) {
                      return xx._id.worker_id.equals(x.id);
                    })[0].leader.fullname;

                  })
                });


                return gres.ok(results);
              });

          });
        });

      });
    }
\$\endgroup\$
1
\$\begingroup\$

First, you should fix your indentation and styling, I would use a tool like JSFiddle's Tidy function to clean up your code.


err:

The word you're looking for is error, no need to sacrifice readability for two characters.

.exec(function(err, found) {

Overriding err/error:

In the following code block, you overwrite the parameter err or error, this is bad practice, it'd be better off named something like error_two.

          collection.aggregate(query).toArray(function(error, results) {
              if (error) return res.serverError(error);

              Worker.find({
                      _id: {
                          $in: workers
                      }
                  })
                  .populate('leader')
                  .exec(function(error, res) {

In fact, you actually overwrite it three times!


Overriding x:

You override x in the following code block:

You should apply the same as before and name it xxx or something (because xx is already existent)

results.map(function(x, y) {
    x.data.map(function(xx, yy) {

        xx.leader_name = res.filter(function(x) {

y:

In the above point's code, you never actually use y, as it is the second parameter, it can be entirely removed, JavaScript does not require an explicit amount of parameters.


In total:

In total, it would look like:

getGroupInRange: function(req, gres) {

    console.log(req.params);

    Group.find({
        name: {
            $in: req.params.id.split(',')
        }
    })
    .populate('workers')
    .exec(function(error, found) {
        if (error) return gres.serverError(error);
            var workers = [];
            found.forEach(function(x) {
                x.workers.forEach(function(xx) {
                    workers.push(xx.id);
                });
            });
            workers = workers.map(function(x) {
                return new ObjectId(x);
            });

            Event.native(function(error, collection) {
                if (error) return gres.serverError(error);

                var query = [{
                    $match: {
                        probe_time: {
                            $gte: new Date(req.params.from),
                            $lt: new Date(req.params.to)
                        },
                        worker_id: {
                            $in: workers
                        }
                    }
                }, {
                    $group: {
                        _id: {
                            worker_id: "$worker_id",
                            group: "$group"
                        },
                        "total_logged_time": {
                            $sum: "$duration"
                        },
                        "total_break_time": {
                            $sum: {
                                $cond: [{
                                    $eq: ["$user.work_mode", 'break']
                                }, "$duration", 0]
                            }
                        },
                        "total_nonidle_duration": {
                            $sum: {
                                $cond: [{
                                    $eq: ["$user.presence", 'active']
                                }, "$duration", 0]
                            }
                        },
                        "total_idle_time": {
                            $sum: {
                                $cond: [{
                                    $eq: ["$user.presence", 'idle']
                                }, "$duration", 0]
                            }
                        },
                        "total_pro_apps_time": {
                            $sum: {
                                $cond: [{
                                    $eq: ["$app_category", 'productive']
                                }, "$duration", 0]
                            }
                        },
                        "total_nonpro_apps_time": {
                            $sum: {
                                $cond: [{
                                    $ne: ["$app_category", 'productive']
                                }, "$duration", 0]
                            }
                        },
                        "status1": {
                            $sum: {
                                $cond: [{
                                    $eq: ["$user.work_mode", 'CUSTOM_1']
                                }, "$duration", 0]
                            }
                        },
                        "status2": {
                            $sum: {
                                $cond: [{
                                    $eq: ["$user.work_mode", 'CUSTOM_2']
                                }, "$duration", 0]
                            }
                        },
                        "status3": {
                            $sum: {
                                $cond: [{
                                    $eq: ["$user.work_mode", 'CUSTOM_3']
                                }, "$duration", 0]
                            }
                        },
                        "status4": {
                            $sum: {
                                $cond: [{
                                    $eq: ["$user.work_mode", 'CUSTOM_4']
                                }, "$duration", 0]
                            }
                        },

                        "leader_name": {
                            $last: "$leader_name"
                        },
                    }
                }, {
                    $project: {
                        "total_logged_time": 1,
                        "total_break_time": 1,
                        "total_nonidle_duration": 1,
                        "total_idle_time": 1,
                        "total_pro_apps_time": 1,
                        "total_nonpro_apps_time": 6388,
                        "print_qty": 1,
                        "medium_print_qty": 1,
                        "medium_download_size": 1,
                        "effectivity": 1,
                        "status1": 1,
                        "status2": 1,
                        "medium_break_time": {
                            $multiply: [3600, {
                                $divide: ["$total_break_time", "$total_logged_time"]
                            }]
                        },
                        "worker_name": "$_id.worker_id",
                        "leader_name": 1
                    }
                }, {
                    $group: {
                        _id: "$_id.group",
                        data: {
                            $push: "$$ROOT"
                        }
                    }
                }];


                collection.aggregate(query).toArray(function(error_two, results) {
                    if (error_two) return res.serverError(error_two);

                    Worker.find({
                        _id: {
                            $in: workers
                        }
                    })
                    .populate('leader')
                    .exec(function(error_three, res) {
                        results.map(function(x) {
                            x.data.map(function(xx) {
                                xx.leader_name = res.filter(function(xxx) {
                                    return xx._id.worker_id.equals(xxx.id);
                                })[0].leader.fullname;
                            })
                        });
                        return gres.ok(results);
                    });
            });
        });
    });
}
\$\endgroup\$

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.