Skip to content
This repository has been archived by the owner on Apr 24, 2020. It is now read-only.

Type may now be either a String or a Number. #175

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Schoonology
Copy link

This is what the docstring for the Socket constructor claims, and is now correct.

Side Effect:
There is now a _type property on all returned Socket objects. _type is always a Number, corresponding directly to the ZMQ enum.

I could have opted to use the existing type property this way, but that seems of minor importance, and I decided compatibility with existing expectations was most important. If that should be changed, let me know.

This is what the docstring claims, and is now correct.
@@ -99,7 +99,8 @@ function gcWrap (self) {}

function Socket(type) {
this.type = type;
this._zmq = new zmq.Socket(defaultContext(), types[type]);
this._type = typeof type === 'number' ? type : types[type];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so _type is the numerical version of type?
I reckon it makes sense to rename that a little bit to reflect that difference?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need 2 properties and make this.type always be string, and this._type (I propose this.inttype) always be int.
Then we could make fill the both fields disregarding of what has been passed as the type argument.

// normalize this.inttype
this.inttype = typeof type === 'number' ? type : types[type];
// normalize this.type
Object.keys(types).forEach(function(type) {
  if (types[type] == this.inttype) { this.type = type; }
}, this);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be fair, I see most code using zmq just opting for the string version of Socket, and that might be a "nicer" method regardless.

In that case, I'd remove support for initializing from a Number entirely, opting to only use Socket(String).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Schoonology actually according to http://api.zeromq.org/3-2:zmq-socket initializing socket with int seems more correct.

@ronkorving ronkorving mentioned this pull request Jun 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants