-
Notifications
You must be signed in to change notification settings - Fork 3
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
Structs from runng do not implement Debug
and might benefit from some other nice Traits.
#25
Comments
I forgot to mention. Sometimes you have to explicitly set structs to be not |
This one is trickier. I've been using Most of the other structs are wrappers around C FFI bindings and not safe for Copy/Clone. Pretty much all of them have a function that needs to be called on
For example, once you've called create_async_context() I don't think it's safe to use the socket (because nng_aio will be using it), so the socket should probably get "consumed" (i.e. But I think you're right that there could be some improvements:
|
Thanks for looking into this. This makes me wonder, in async code it is common to use closures and thus often one needs to "clone" things. In the little test app I posted, I thus end up cloning a |
It's ok to clone it, it won't get closed until the last reference goes away. The At the moment there's only let a = factory.pair_open()?.listen(&url)?;
let b = factory.pair_open()?.dial(&url)?;
let a_thread = thread::spawn(move || -> NngReturn {
let mut ctx = a.create_async_context()?;
//...
}); I don't plan to use the synchronous functions like |
…s, PartialEq on NngString/Msg, and Clone on sockets (#25) - https://rust-lang-nursery.github.io/api-guidelines/interoperability.html#types-eagerly-implement-common-traits-c-common-traits
…s, PartialEq on NngString/Msg, and Clone on sockets (#25) - https://rust-lang-nursery.github.io/api-guidelines/interoperability.html#types-eagerly-implement-common-traits-c-common-traits
* Numerous improvements to Result and error-handling - Rename NngResult to more canonical `Result` and get rid of NngReturn - Rename NngFail to more canonical runng::Error - TryFrom conversion for NngFail - impl std::error::Error (fix #24) - Err and Unknown enums now Errno and UnknownErrno, respectively. - Rename `NngFail::succeed_then()` to `zero_map()` - Zero is `Ok` and then it's the same as `Result::map` - Tests use fewer glob (*) imports - impl AsRef/AsMut<[u8]> for Alloc and NngMsg (#25) - https://rust-lang-nursery.github.io/api-guidelines/interoperability.html#conversions-use-the-standard-traits-from-asref-asmut-c-conv-traits
Right now if a struct contains for example a
protocol::pair1::Pair1
it can't deriveDebug
becausePair1
doesn't implement it. It is generally accepted to more or less always derive or implement at leastDebug
for allpub struct
s.Others that might be useful can be found in the rust book.
Eq
might also make sense for a socket. For the difference betweenEq
andPartialEq
, see here.Some traits are handy if it makes sense, like
Copy
, but they have implications for the future. If you later add a non-Copy
property to your struct, you will have to break the API by removingCopy
and instances will be implicitly copied on the stack which might not always be a good idea. HoweverClone
is explicit and might be a good idea.The text was updated successfully, but these errors were encountered: