Node/Javascript with Revealing Module Pattern

Question

I am playing around with node/javascript. This is just a simple code, logic is not important just playing with the EventEmitter. Can someone please tell me what I am doing wrong in terms of code not the logic.

Thanks for the help.

  var emitter = require('events').EventEmitter;
var http = require('http');


function github()
{
    var emitterInstance = new emitter();

    this.getEmitter = function()
    {
        console.log('returning the emitterInstance');
        return emitterInstance;
    };

    this.getData  = function()
    {
          http.get("https://api.github.com/users/loneshark99/gists", function(error,resp)
          {
                if(error)
                {
                    console.log('Error occured!');
                }
                else
                {
                    emitterInstance.emit('dataReceived');
                }
          });
    };

    return {
    emt : getEmitter,
        getData : getData
    };
}

var g = new github();
g.emt().on("dataReceived", function() { console.log('data received from github')})
g.getData();

Error is :

D:\test.js:33
        getData : getData
                ^
SyntaxError: Unexpected token :
    at exports.runInThisContext (vm.js:73:16)
    at Module._compile (module.js:443:25)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:501:10)
    at startup (node.js:129:16)
    at node.js:814:3

Answer :: This works

var emitter = require('events').EventEmitter;
var http = require('http');


function github()
{
    var emitterInstance = new emitter();

    this.getEmitter = function()
    {
         console.log('returning the emitterInstance');
         return emitterInstance;
    };

    this.getData  = function()
    {
           http.get("http://pluralsight.com", function(resp)
           {
                emitterInstance.emit('dataReceived');
           });
    };


    var obj =  { emt : getEmitter, getData : getData };

    return obj;
}

var g = github();
console.log(g);
g.emt().on("dataReceived", function() { console.log('data received from github')})
g.getData();

@Ben you've basically said "I have this code, what's wrong with it?" Obviously something is wrong with it, otherwise you wouldn't be posting this question. What is it doing that makes it... "wrong". is it throwing an error? producing unexpected results? producing no results? (no results would likely mean you haven't done enough debugging..) — Kevin B, May 26 '15 at 20:43
@Ben why do you need a `return {...}` when you are already attaching the functions to the object instance. `getData != this.getData` — PSL, May 26 '15 at 20:54
@PSL I was trying to use the revealing module pattern to do the Public/Private, in this case everything is public but I was just trying to use that pattern — Ben, May 26 '15 at 20:58
Please stop to constantly change the code in your question (except for minor typos that are unrelated to the subject), it invalidates the answers. If you've got a new problem, ask a new question. — Bergi, May 26 '15 at 21:11

score 2 · Accepted Answer · edited May 23 '17 at 10:26

There are many problems with your code:

var emitter = require('events').EventEmitter;

I would recommend to follow JavaScript naming conventions, and capitalise constructor names. Emitter it should be.

this.getEmitter = function() {

Your function seems to be supposed to be a factory, not a constructor function. For the revealing module pattern, you use local declarations for your functions and variables, instead of attaching properties to this. You just return an object literal with the exported values in the end.

emitterInstance.emit('dataReceived');

Since the purpose of your code seems to be that data can be received, you should actually propagate the received data. Hand it back to your controller by passing the resp as an argument to the emitted event. Make this a var getEmitter = function() {, or even better a function getEmitter() {.

return 
{

ASI will hit you here. Hit you hard. Don't use Allman bracket style in JavaScript, but put your braces on the same line.

emt : getEmitter,

This doesn't match your invocation. Should the method name really be emt?

new github();

As said above, github is a factory function that does return an object, not a constructor that creates instances which inherit from a prototype. Don't use new here, just call the function.

g.getEmitter.On

To get your event emitter instance, you will need to call your method. And On should be on.

I fixed the typo for on. and thanks for the conventions. appreciate it — Ben, May 26 '15 at 20:59
Ah, thanks for including an error message in the question. That error seems to be appropriate for the line-break case, as `emt:` is a valid label but `getData` after a comma is no more. If you have fixed the ASI, I cannot explain this any more, make sure that you properly saved your file and node doesn't cache the module source. — Bergi, May 26 '15 at 21:05
I think you were correct, when I added the return to one line returned the object, it worked. Seems fishy to me, why one works and not the other. — Ben, May 26 '15 at 21:23
That's why I have linked that other question for you :-) Also have a look at [these](https://stackoverflow.com/search?q=ASI+return+%5Bjavascript%5D) if you need further reading. — Bergi, May 26 '15 at 21:27

score 1 · Answer 2 · answered May 26 '15 at 20:30

1

The problem is new github() returns this. Try:

var g = github();

I would double check and make sure emitter is meant to be newed up.

answered May 26 '15 at 20:30

beautifulcoder

10,832
3
19
29

score 0 · Answer 3 · answered May 26 '15 at 20:34

0

getEmitter should be getEmitter(), but even doing that would be calling it out of scope. You could assign those functions to the prototype of github like so:

this.getEmitter = function() {
  console.log('returning the emitterInstance');
  return emitterInstance;
};

Also, On should be on.

The same would need to be done to the g.getData() call.

An alternative would be to define it like:

github.prototype.getEmitter = function() { ... }

answered May 26 '15 at 20:34

Evan Lucas

622
3
6

I understand that it can be done with prototype or this keyword. I am trying to understand what is wrong in the code. – Ben May 26 '15 at 20:41
`g.getEmitter` is not defined. `g.getData` is not defined. – Evan Lucas May 26 '15 at 20:49
Exactly why is my question, wont they get wrapped – Ben May 26 '15 at 20:51
If you `console.log(g)` you should get undefined. Move the opening curly brace onto the same line as return – Evan Lucas May 26 '15 at 20:53

Node/Javascript with Revealing Module Pattern

3 Answers3