Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Events need a different approach #231

Closed
mneri opened this issue Apr 29, 2014 · 0 comments
Closed

Events need a different approach #231

mneri opened this issue Apr 29, 2014 · 0 comments

Comments

@mneri
Copy link

mneri commented Apr 29, 2014

I think events are written the bad way. When a line is received the 'raw' event is emitted. The Client object listens on the 'raw' event, handles messages with a huge switch and then re-emits the event to his listeners.

// from irc.js
switch (message.command) {
case 'PING':
    // stuff
    self.emit('ping', /* params */);
    break;
case 'NOTICE':
    // stuff
    self.emit('notice', /* params */);
    break;
// And many, many others...
}

The user to listen to a NOTICE have to write something like:

client.on('notice', function(from, to, text) {
    // stuff
});

Not all messages are handled this way. The "rare" ones are not handled at all and you should listen to the 'raw' event and write the huge switch again. For example:

client.on('raw', function(message) {
    switch(message.command) {
    case 'RPL_NOWAWAY':
        // stuff
        break;
    case 'RPL_UNAWAY':
        // stuff
        break;
    }
    // and many, many others...
});

Now, my point is that there are two different interfaces for the same thing. The first is client.on('command', ...) the second is client.on('raw', ...) and, in my opinion, the difference is not subtle. It would be great if every type of message was fired with the first interface. JavaScript is very powerful and this change is easy to make:

// from irc.js
self.conn.addListener("data", function (chunk) {
    // [...]
    var message = parseMessage(line, self.opt.stripColors);
    try {
        // This is where the 'raw' event is emitted.
        // self.emit('raw', message);
        var args = [message.command.toLowerCase()]
             .concat([message.nick])
             .concat(message.params);
        self.emit.apply(self, args);
    } catch ( err ) {
        // [...]
    }
});

Now every type of message can be handled the same way. For example:

client.on('rpl_away', function() {
    // stuff
});

client.on('privmsg', function(from, to, text) {
    // stuff
});

The first parameter to the callback is always the sender of the message. The subsequent parameters are the original parameters of the message. With this change the number of lines will be greatly reduced but probably it will break the actual interface of the library. This is why I wrote an issue and not a push request.

Kindly,
Massimo

@mneri mneri changed the title Events need a different approach. Events need a different approach Apr 29, 2014
@osslate osslate closed this as completed May 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants