-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Implement a multimap #784
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
Comments
This addresses the following feeback: Your code looks really good, and very Rustic. I only have a few comments: · In ProcessRecord you use isize for the pid and ppid. These aren’t really sizes, so I’d go with i32 · Line 102: since all you’re doing is panic!() on Err, you can just use .unwrap() · Line 107: you could use “filter_map” instead of filter and then “collect” the results. This eliminates the need for “mut records” on line 98 · Line 126: use “filter_map” · I’d add Cargo support · You could consider using a map type containing vector types for storing the ppid->pid translation. That would improve performance since the record list wouldn’t getting iterated over multiple times. Unfortunately Rust doesn’t have a multimap yet (see rust-lang/rfcs#784), otherwise you could just use that! Using a map type for the lookup is not supported with this but will be evaluated.
It looks like there's a pretty decent go at this already at https://github.com/havarnov/multimap |
Should this be closed? |
Please No. I don't want to use external. If rust provides and supports or even merge this project then we can use it. external crate may die anytime as mostly one guy support so for core features like this one there should be some internal support. Almost all langs supports this. |
cc @rust-lang/docs Perhaps it would be useful to have, somewhere in a FAQ, an explanation of how external crates are not bound to be of worse quality and less maintained than |
@codeyash Are you sure "almost all langs" do in fact support that? None of Python, D, Java, Ruby, Go, PHP, Javascript, Perl, or Swift provide multimaps in their standard libraries. |
@codeyash: Qt is not a language; it's a library written in C++. A Rust equivalent would be on crates.io, not in the stdlib; just as Qt is available as a separate package, and not as part of On the other hand, in Rust, a library like Qt would almost certainly be split out into multiple crates; much as Qt has been splitting itself first into separate libraries for different functionalities (Qt 3 -> Qt 4) and then further into separately-installable packages (Qt 4 -> Qt 5). In short: If you don't find using Qt for its multimap objectionable in the C++ world, there is no defensible reason to object to using a Cargo crate for it in the Rust world. (Heck, Rust makes it easier, because we have a powerful dependency manager that C++ lacks. Getting Qt set up for development can be painful unless you either use a distro package or use a Qt-themed development environment.) |
@eternaleye I admire your points. Its seems you are saying everyone is walking in one single straight line and we should also in order to maintain rules. :) Qt is totally different topic. I'm advance user. QMultimap, QHashMap are really important packages. As per Qt we should use Qt libraries if available and not plain c++. Qt Multimap can't be used without Qt. Qt maintains its framework. Qt is not just library. It contains full eco system like compiler, moc objects etc anyways lets leave Qt out from this discussion. Everyone is ready to debate and told me to follow trends and do what others are doing. This is not engineering. This is something else. Be unique. Rust is unique in its style and approach. I really appreciate creators thinking. Many guys debate with me why OOPS better and rust lacks. All these guys want to see traditional and wants to follow what is everybody is following. I don't. I raised a bad point other langs are using. I'm taking my words back. Forget that point. Why we should or shouldn't include here, please add points.
|
@codeyash: To address your points one by one:
|
As an implementation exists in https://crates.io/crates/multimap and T-libs did not seem interested in moving this into the standard library I am closing this. |
Monday Jul 14, 2014 at 20:12 GMT
For earlier discussion, see rust-lang/rust#15669
This issue was labelled with: A-libs, I-enhancement in the Rust repository
Extensions to
treemap
andhashmap
, with set variations. Indicated by the wiki.The text was updated successfully, but these errors were encountered: