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

make subscribe with context less error prone #12

Open
mwaschkowski opened this issue Jun 13, 2014 · 5 comments
Open

make subscribe with context less error prone #12

mwaschkowski opened this issue Jun 13, 2014 · 5 comments

Comments

@mwaschkowski
Copy link

Hi,

A number of times now I've done stuff like this:

radio(constant.SHOW_CLIENT_APPLICATION).subscribe(this.xyz, this);

instead of this:

radio(constant.SHOW_CLIENT_APPLICATION).subscribe([this.xyz, this]);

Would you please create a new method/alias to prevent this kind of thing? Maybe something like:

radio(constant.SHOW_CLIENT_APPLICATION).subscribe(this.xyz).context(this);

I know its minor, but its a big problem when you first run into this as there are no error/indications to help out.

Thank you,

Mark

@uxder
Copy link
Owner

uxder commented May 22, 2015

Yeah, I look at it now and I agree that pattern of using an array was array odd and I would be open to changing it. Personally, I don't really use context and just simply use a bind method.

IE: radio(CHANNEL).subscribe(angular.bind(this, this.mymethod);

@mwaschkowski
Copy link
Author

Ah, I see. I don't use angular, so don't have the bind method available. I
would just make a different method name that took the two parameters.
subscribeBind(x, y) maybe?

Regards,

Mark

On Fri, May 22, 2015 at 6:20 PM, Scott (Tomoharu) Murphy <
[email protected]> wrote:

Yeah, I look at it now and I agree that pattern of using an array was
array odd and I would be open to changing it. Personally, I don't really
use context and just simply use a bind method.

IE: radio(CHANNEL).subscribe(angular.bind(this, this.mymethod);


Reply to this email directly or view it on GitHub
#12 (comment).

@uxder
Copy link
Owner

uxder commented May 23, 2015

Angular was just an example but Bind is available in ES5 so if you just need to support IE9 and up, it is natively available already. http://itsnull.com/articles/start-using-es5/ (check Function.prototype.bind). If you need to support older browsers, there are plenty of bind solutions like lowdash/underscore e.t.c.

For radio and making it better, I think we can just keep it simple and an provide an alternative to that .subscribe([method, context]) and make the following available:

radio(constant.SHOW_CLIENT_APPLICATION).subscribe(this.xyz, this);

which is much more natural.

@mwaschkowski
Copy link
Author

Yes, sounds good!

Mark

On Sat, May 23, 2015 at 5:34 PM, Scott (Tomoharu) Murphy <
[email protected]> wrote:

Angular was just an example but Bind is available in ES5 so if you just
need to support IE9 and up, it is natively available already.
http://itsnull.com/articles/start-using-es5/ (check
Function.prototype.bind). If you need to support older browsers, there are
plenty of bind solutions like lowdash/underscore e.t.c.

For radio and making it better, I think we can just keep it simple and an
provide an alternative to that .subscribe([method, context]) and make the
following available:

radio(constant.SHOW_CLIENT_APPLICATION).subscribe(this.xyz, this);

which is much more natural.


Reply to this email directly or view it on GitHub
#12 (comment).

@uxder
Copy link
Owner

uxder commented May 24, 2015

Okay, will try to find time for this sometime. But if you have time, feel free to tackle it as well.

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