Skip to content

Conversation

@Lellansin
Copy link

It's better to use this.prefix than an upper scope's prefix.

@Lellansin Lellansin changed the title use prefix of this itself Use prefix of this itself Nov 26, 2015
@objectiveSee
Copy link
Contributor

I was thinking we could add a proto function that is the sole place responsible for generating a channel name, such as :

Redis.prototype.channelName = function(room){
  var channelName = this.prefix + '#' + this.nsp.name + '#';
  if ( room ) {
    channelName +=  room + '#';
  }
  return channelName;
}

There are currently 5 or so places in the code that do string concatenation to form Redis channel names.

@nkzawa
Copy link
Contributor

nkzawa commented Dec 14, 2015

I think we can just append namespace to prefix in the first place like

this.prefix = prefix + '#' + this.nsp.name + '#';

@Lellansin
Copy link
Author

@nkzawa Looks good 👍

@objectiveSee
Copy link
Contributor

@Lellansin can you resolve the merge conflicts?

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

Successfully merging this pull request may close these issues.

3 participants