search

J0 / phoenix_gen_socket_client

Socket client behaviour for phoenix channels
MIT License
232 stars 48 forks source link

add GenSocketClient.topic_status/1 function (and tests) to query the … #27

Closed boydm closed 6 years ago

boydm commented 6 years ago

I had a need to query the status of a topic, without actually sending a ping to the server. Querying the process dictionary to get the value of the join_ref in my socket module would work, but means leaking "private" knowledge of phoenix_gen_socket_client internals into the callback module.

So I added a very simple GenSocketClient.topic_status/1 function, which queries the join_ref and returns either :joined or :not_joined.

This allows me to check the status of a topic, while keeping the knowledge of how to query the process dictionary in the phoenix_gen_socket_client where it belongs.

Also added tests.

sasa1977 commented 6 years ago

Thanks for contributing back!

This looks good, and I'm basically in favour of merging this feature, but I'd like to discuss one thing about the interface first.

What is the reason for choosing this particular return type? Since the topic status is binary (joined/not joined), we could return a boolean, and of course rename the function to e.g. joined?. At first glance, that seems a bit more idiomatic to me. What do you think?

boydm commented 6 years ago

Yes. Good point.

The signature should be joined?/1 and return a boolean.

I'll update the pull request later today after I'm done driving the kids around...

sasa1977 commented 6 years ago

Thanks!

While you're at it, could you also capitalize the first letter in the function doc? I believe this is the only other remark I have :-)

boydm commented 6 years ago

Right. Changed the name of the function and fixed the capitalization.

sasa1977 commented 6 years ago

Thanks!

I made a couple of minor changes, and merged.

boydm commented 6 years ago

Thank you very much!

Your changes area also a good example of keeping internal knowledge internal. Makes sense.

--Boyd