5
\$\begingroup\$

Background

I have a situation where I have fixed number of objects with their own routines that would need to be called with the same initial message.

To accomplish this, I have a Factory that binds objects to a Composite object whose main method is to call methods on its subordinates without having to know how many it has.

This routine is to be called upon receiving a message from a queue, the client to that queue is responsible for initiating that procedure.

Code/ Question

Just to keep the code looking succinct, here are the individual modules for completeness.

//-------------------------------------------------------------------
// module1/index.js
//-------------------------------------------------------------------

function A () {}

A.prototype.method = function(msg, cb) {
  console.log('A class: %j', msg );
  cb(null, 'done message');
};

//-------------------------------------------------------------------
// module2/index.js
//-------------------------------------------------------------------

function B () {}

B.prototype.method = function(msg, cb) {
  console.log('B scraper: %j', msg);
  cb(null, 'done message');
};

Here is the factory that has the job of knowing what it imports and attaching the objects defined above to the Factory's caller:

//-------------------------------------------------------------------
// - factory.js
//-------------------------------------------------------------------

var A = require('./module1');
var B = require('./module2');

function Factory () {
  var o = Object.create(null);
  o.obj1 = new A();
  o.object2 = new B();
  return o;
}

Now here's the part that is in question. The method method does some 'clever' stuff to make something that the async library can understand and use. This code runs, and it about as efficient as it needs to be but I would like to ask if there is another, more readable way to build an object for use in async.parallel().

//-------------------------------------------------------------------
// - composite.js
//-------------------------------------------------------------------
var async = require('async');

function Composite () {
  this.objects = new Factory();

}

Composite.prototype.method = function(msg) {
  var self = this;
  var enum = Object.keys(this.objects);
  var newObj = {};
  // for each member-name in the obj.
  enum.forEach(function (name) {
    // put a function in the newObj
    newObj[name] = function (callback) {
      // who calls the object's method with some initial values
      // and a callback
      self.objects[name].method(msg, function(err, results){

        if (err) { callback(err); }
        callback(null, results);

      });

    };
  });
  // actually do something
  async.parallel(
    newObj,
    function(err, results) {
      console.log(results);
  });
};


module.exports = Composite;

This is just here for extra context.

//-------------------------------------------------------------------
// RMQ Client
//-------------------------------------------------------------------
var Composite = require('./composite');
var Client = {};

Client.on('message', startJob, msg)

function startJob (msg) {
  var comp = new Composite();
  comp.method(msg);

}
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! I hope you get some great answers! \$\endgroup\$
    – Phrancis
    Commented Feb 23, 2015 at 17:27

1 Answer 1

2
\$\begingroup\$

There is an error that needs to be handled correctly:

      self.objects[name].method(msg, function(err, results){

        if (err) { 
           return callback(err); 
        }
        return callback(null, results);

      });

You need to return the callback on the error, otherwise if an error comes down, you will hit both callbacks.

\$\endgroup\$
0

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.