Module pattern feedback

[Nabeel S.]

Hi all,

I've been meaning to post this up, as it's a pattern I've been using with any modules that act as a wrapper to something.

I just wanted to get some feedback, as I haven't seen this explicitly anywhere, and I am curious if I may be causing problems down the line.

I have quite a few apps, which all share a common set of libraries, in a package; ie: logging, configuration, database, etc.

I'll take logging as an example, I use bunyan.

exports.init = function(opts) {

   

  // Do something with options

  // There's a bunch of stuff for different streams to different services

  var logger = bunyan.createLogger(...);

  module.exports = logger;

}

Then in the app initialization:

require('package/lib/logger').init({ ... });

And now in some other module:

var logger = require('package/lib/logger');

logger.debug(...);

Basically replacing the exports with the instance, and then it can be called from anywhere, and accessed.

So my question would be - aside from losing the ability to re-initialize (these should theoretically only be initialized once) - is there any downside to this pattern; in terms of memory or efficiency?

Nabeel

[greelgorke]

there is nothing to complain about, aside of self modifying code and violation of the principle of least astonishment. but there are (at least) 2 other ways to do this without code modification:

1.

module.exports = function(opts){

  if( ! isInitialized ){

    // do something with opts

   // if initalization is mandatory do defaults silently or throw an error 

  }

  

  return bunyan.createLogger() 

}

var logger = require(./logger)(opts)

2.

module.exports = function(opts){

  if( ! isInitialized ){

   // if initalization is mandatory do defaults silently or throw an error 

  }

  

  return bunyan.createLogger() 

}
module.exports.init = function() {

  if( ! isInitialized ){

    // do something with opts

  }

}
var logger = require(./logger)()

logger.init(opts)

if you want less parens on user side you could make your logger function polymorphic, but it may be too confusing.

[Ilya Sh.]

I would go with structure greelgorke suggested. It's less tricky and, IMO, more structured. I've been writing my modules in the same way

function Mod () {}

Mod.prototype.init = function(){}

module.exports = Mod;

This way you can either have multiple instances of Mod or you can make it a singleton with something like

var mod = new Mod;

module.exports = mod;

or any other way you like.

[Rick Waldron]

I might be half-trolling here...

On Fri, Mar 22, 2013 at 1:50 AM, Nabeel S. <nsha...@gmail.com> wrote:

  var logger = bunyan.createLogger(...);

Why do module authors make gross factory APIs?

Instead of wrapping the export in this icky "createLogger" method, that doesn't do _anything_ (one assumes a factory exists to provide some additional layer of logic over the base constructor, or else what is the point?), why not export the constructor?

var logger = new bunyan.Logger();

Yay, much nicer.

Feel free to completely ignore me.

Rick

[Mikeal Rogers]

var logger = bunyan();

:)

-Mikeal

--
--
Job Board: http://jobs.nodejs.org/
Posting guidelines: https://github.com/joyent/node/wiki/Mailing-List-Posting-Guidelines
You received this message because you are subscribed to the Google
Groups "nodejs" group.
To post to this group, send email to nod...@googlegroups.com
To unsubscribe from this group, send email to
nodejs+un...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/nodejs?hl=en?hl=en
 
---
You received this message because you are subscribed to the Google Groups "nodejs" group.
To unsubscribe from this group and stop receiving emails from it, send an email to nodejs+un...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Ilya Shaisultanov]

Weird

module.exports = Logger;
// ...
module.exports.createLogger = function createLogger(options) {
  return new Logger(options);
};

Maybe compatibility with old API?

[Mikeal Rogers]

i was calling out a more ideal API, not what is actually implemented.

[Ilya Shaisultanov]

Right, I just looked at the source out of curiosity.

[Rick Waldron]

On Sat, Mar 23, 2013 at 8:25 PM, Mikeal Rogers <mikeal...@gmail.com> wrote:

var logger = bunyan();

I was taking into account that bunyan doesn't drink single exports kool-aid (which makes no difference to me)

Either way, yes, that _is_ simpler...

Rick

[Rick Waldron]

On Sat, Mar 23, 2013 at 8:29 PM, Ilya Shaisultanov <ilya.sha...@gmail.com> wrote:

Weird

module.exports = Logger;
// ...

module.exports.createLogger = function createLogger(options) {

  return new Logger(options);


};

Maybe compatibility with old API?

Hilarious. I control-F'ed for "createLogger". All of the README uses "createLogger". Makes me happy to see that I was at least half wrong and also that you could use Mikeal's suggestion for "path of nicest api".

Anyway, now I'm wondering why module.exports is being set to the constructor function object itself, AND THEN expando properties are defined. Why not define them as static properties on Logger and have _one_ export?

Again, feel free to ignore me. (I'm turning into Statler and (or) Waldorf).

Rick

[Nabeel S.]

Thanks for all the feedback guys, still trying to learn all the intricacies of the module system, and just the best patterns to use. I remember reading somewhere (I'll have to dig it up, though Crockford comes to mind), that the the "createLogger()" type of pattern is the preferred one. I see it in a lot of libraries. Never really saw the point in that myself, seems more restrictive. 

What I basically want to do is have the main exports be the instantiated object (of anything), but a static init() function. Just gotta play with all your suggestions and see how it works. Thanks guys!

You received this message because you are subscribed to a topic in the Google Groups "nodejs" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/nodejs/peTzWLT9gpQ/unsubscribe?hl=en.
To unsubscribe from this group and all its topics, send an email to nodejs+un...@googlegroups.com.

[Rick Waldron]

On Sat, Mar 23, 2013 at 9:26 PM, Nabeel S. <nsha...@gmail.com> wrote:

Thanks for all the feedback guys, still trying to learn all the intricacies of the module system, and just the best patterns to use. I remember reading somewhere (I'll have to dig it up, though Crockford comes to mind), that the the "createLogger()" type of pattern is the preferred one.

I don't have empirical data to back this claim, but I strongly doubt that this is preferred. As far as I'm concerned, a factory that does nothing but wrap instantiation is a bug.

 

I see it in a lot of libraries. Never really saw the point in that myself, seems more restrictive. 

It's cruft.

 

What I basically want to do is have the main exports be the instantiated object (of anything), but a static init() function. Just gotta play with all your suggestions and see how it works. Thanks guys!

Why do you need the init()? Why not... 

// log.js

// just export an instance. done. 

export = require("bunyan")();

// In some other file...

var log = require("log");

log.info("blah blah blah");

(this suggestion is untested)

Rick

[Nabeel S.]

Why do you need the init()? Why not... 

// log.js

// just export an instance. done. 

export = require("bunyan")();

// In some other file...

var log = require("log");

log.info("blah blah blah");

(this suggestion is untested)

Well, the logger was just a generalized demo of the concept. I do the same thing for connections to cassandra, redis, etc. In the specific case of the logger, the initialization sets up a zeromq stream for the logs and specific messages, and also for stats and things.

Something I've been looking for is a guide of bad practices. There are a bunch about good practices and conventions, but when straying from those, you don't want to come to the realization later on that... "I've made a huge mistake..."

[substack]

On Saturday, March 23, 2013 5:22:12 PM UTC-7, Rick Waldron wrote:

Why do module authors make gross factory APIs?

Instead of wrapping the export in this icky "createLogger" method, that doesn't do _anything_ (one assumes a factory exists to provide some additional layer of logic over the base constructor, or else what is the point?), why not export the constructor?

var logger = new bunyan.Logger();

Yay, much nicer.

It's something of an imposition on downstream consumers of a library to need to remember which functions return objects and which instantiate instances. I like it much better when modules just expose functions that return objects. Whether or not some "constructor" or "factory" is at work is not something that I as a module consumer should need to care about because that is an implementation detail that the module author should be concerned with, not me as the consumer.

[Scott González]

On Sat, Mar 23, 2013 at 8:22 PM, Rick Waldron <waldro...@gmail.com> wrote:

On Fri, Mar 22, 2013 at 1:50 AM, Nabeel S. <nsha...@gmail.com> wrote:

  var logger = bunyan.createLogger(...);

Why do module authors make gross factory APIs?

Module authors are likely just following core conventions. The reason node originally used these factories was a limitation in the C++ code. However, Isaac has also explained why he prefers not requiring the use of the new keyword.

[Ilya Shaisultanov]

A way around that is

var logger = new (require('logger'));

But that's ugly and, as I found out, jscoverage will strip the parenthesis and break the code.

It's really, IMO, about consistency, if you export constructors in half the modules and factories in others that's not going to help. Isaac's approach is nice as long as developers follow it with their modules.

[greelgorke]

i'm with substack here. my arguments on this are:

the HOW instances are created and IF it's a singleton or not are, in fact, implementation details and should be hidden most of the time from the user. 

when i var foo = require('something') , then it's not obvious, that this is a constructor. a factory function is in 99% of the time clear.

factory functions help to abstract away the details, so it is easier to exchange the hidden parts. This argument is not that important in js as it is in static typed langs, but still valid.

on the other side i also think Rick and Mikeal are right regarding style and amount of code to maintain. a thing like that:

module.exports = function createLogger(){ return require('customLogger').createLogger()}

is awful. a factory function should definitely do something more than just return it's product.

and about static init, what i did about it in a lib: the main entrypoint of the whole package was a function which took configuration and returned a factory function. the user required the package once and re-exported the returned factory function. usage:

reexport.js:

module.exports = require('thelib')({/*annoying options*/})

myLogic.js:

var someWorkingFunc = require('./reexport')()

the user may one or more independently configured factories of the lib and use them in different ways. (and can wrap it's own favorite api-style around mine for internal code.)

PS: this is also one way to do simple DI:

module.exports = function entry(preferredChannel){ 

  preferredChannel = preferredChannel || fs.createWriteStream('/var/log/myapp')

  return function getLogger(loggerLevel){return new AwesomeLogger(preferredChannel, loggerLevel)}