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

Return type of sock.get(zmq::sockopt::type) #522

Open
jasujm opened this issue Oct 15, 2021 · 2 comments
Open

Return type of sock.get(zmq::sockopt::type) #522

jasujm opened this issue Oct 15, 2021 · 2 comments

Comments

@jasujm
Copy link
Contributor

jasujm commented Oct 15, 2021

The new sock.get(zmq::sockopt::option) API does a good job in retrieving socket options in a typesafe manner, and is definitely an improvement over the old sock.getsockopt() API.

But I think that sock.get(zmq::sockopt::type) in particular works illogically. Currently, this won't compile:

$ cat test.cc 
#include <zmq.hpp>

#include <cassert>

int main()
{
    zmq::context_t ctx;
    zmq::socket_t sock(ctx, zmq::socket_type::push);
    assert(sock.get(zmq::sockopt::type) == zmq::socket_type::push);
    return 0;
}
$ g++ -c test.cc 
In file included from /usr/include/c++/10/cassert:44,
                 from test.cc:3:
test.cc: In function ‘int main()’:
test.cc:9:41: error: no match for ‘operator==’ (operand types are ‘int’ and ‘zmq::socket_type’)
    9 |     assert(sock.get(zmq::sockopt::type) == zmq::socket_type::push);
      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~ ~~~~~~~~~~~~~~~~~~~~~~
      |                    |                                         |
      |                    int                                       zmq::socket_type

Would it make more sense that in C++11 and above sock.get(zmq::sockopt::type) would return enumerator from zmq::socket_type instead of an int? int is neither typesafe nor used to represent socket types elsewhere in the library. This change would break backward compatibility slightly, but in a way that I'm sure any user of zmq::sockopt::type would accept.

What do you think?

@gummif
Copy link
Member

gummif commented Oct 16, 2021

Yes it looks like we overlook that case. It would be easy to add a new socket_type case to the sockopt enum that would return socket_type on get.

@jasujm
Copy link
Contributor Author

jasujm commented Oct 21, 2021

That sounds good :). I created a PR which would do that: #523

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