zeromq / zmqpp

0mq 'highlevel' C++ bindings
http://zeromq.github.io/zmqpp
Mozilla Public License 2.0
439 stars 195 forks source link

zmqpp::socket Send/Receive : using bool dont_block or int flag #58

Closed xaqq closed 8 years ago

xaqq commented 10 years ago

Hi,

As pointed out in #56 the parameters of the send and receive overloads in zmqpp::socket are not very consistent.

Sometimes we use an int flag and in other places we use bool dont_block for options. The zmqpp::message and zmqpp::signal overloads use bool because the only meaningful flag would be block or don't block. send_more is not relevant here (please correct me if i'm wrong).

However, send_more is relevant in the send_raw / receive_raw method, so a int flag is needed.

The std::string overload, which is more of a syntactic sugar, uses int flag. I guess it can be used to send multiple string as 1 message with send_more, but wouldn't using a zmqpp::message be a better alternative?

Changing the std::string overload so that it takes bool dont_block would break compatibility for people using this overload as describe before, but would bring a bit more consistency to the API.

What do you think?

sunkin351 commented 8 years ago

I would welcome this. +1

benjamg commented 8 years ago

We could have both for now, deprecating the old one, then remove it next major version jump? We should probably get better at tagging stable releases anyway :)

benjamg commented 8 years ago

Turns out you can't have both an int and bool version as they'd both have defaults so the over load is ambiguous. Ideally I'd opt for the flag version not to have the default but that would again be breaking.

sunkin351 commented 8 years ago

Isn't the compiler able to deduce the need to convert int to bool and vice versa? Only warnings should be given from the compiler.

benjamg commented 8 years ago

Not if they both have default values for the int / bool section :/ I'm tempted to make the int version not have a default on the basis that anyone using the default can safely use the default for the bool version too. I'm not sure if that counts as a breaking change though.