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

API: Clean up parameters passed to WebSocketClient.prototype.connect #164

Open
theturtle32 opened this issue Nov 20, 2014 · 0 comments
Open
Assignees

Comments

@theturtle32
Copy link
Owner

Continuing discussion that started in #158

IMHO connect() should be the method to "activate" the client, and should be callable just once.

I tend to agree with @ibc here, but I can also understand @braytak's perspective. @braytak - the benefit is simplifying the API. The parameters that can be passed to connect() are a complete mess with multiple optional parameters of various types. Also, since I wasn't specifically planning on having the objects be reusable for multiple connections when I originally wrote the client code, the behavior when doing so is undefined. There may be bugs that arise if the state isn't properly cleaned up or reset before making another connection. Or it may work fine, I don't really know. Since there aren't unit tests in the library for things like this, (something I'd like very much to start adding), it makes me nervous.

Looking back at my original goal of cleaning up the connect() API, what if we change it to accept a single options object parameter, the same format as the constructor. That way, you could set all the options in the constructor if you wanted, and override individual options each time you call connect(). Or don't pass any options to the constructor, and pass them all to connect(). Internally, it would keep the original options from the constructor (or the defaults), and every time connect() was called, a new copy would be created and extended with the newly supplied options.

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant