Associate Container/Set

[choznerol]

Will close part of #4

The code implementation (set/mod.rs) will be push/rebase in thie branch. Besides the code, the draft will be edit in HackMD before ready to publish. Please feel free to add/suggest good topics to cover!
👉https://hackmd.io/VsM13vR1TrKzVRsKpHuTjA?both

[choznerol]

Implement HashSet::union()

In the last commit (caab7fb) I implemented union(&self, other: &HashSet<T>) -> HashSet<T> with following strategy:

1. Clone other into a new set.
2. Insert every item from self to the new set (duplication avoided by insert())

and it works

To achieve this, I had to derive Clone trait for both HashMap and HashSet, and also bound the key T of HashSet with Clone. Before proceeding with other methods like difference(), I'd like to make sure if using Clone this way is reasonable and expected?

[weihanglo]

@choznerol Nice try. Here are my opinions concerning binding HashSet with Clone trait.

First, to be a generic container type, HashMap/HashSet is expected to store nearly every types. Current bound traits on key T in HashMap (Hash, Eq) are reasonable and the intent is explained in the doc. However, bound with Clone trait is too subtle and would lose the flexibility to store arbitrary types. Moreover, clone operation may or may not be expensive but still an overhead.

Second, creating a new HashSet is a rational strategy in dynamic language like Python. In Rust we follow a few essential rules to reduce resource usage. We can

• move values into new HashSet without cloning.
• create unions that borrow from original sets.
• return an lazy iterator representing the union to avoid any extra allocation.

All these methods can cope with the Clone issue.

Last, you can still decide to bind Clone trait if that makes the explanation of HashSet more expressive and understandable. However, it could be more decent if methods which are restricted to Clone trait would be moved into another impl block. I can accept that types without Clone do not have any set operations.

These commits are still awesome attempts. Great job!

[choznerol]

@weihanglo Thanks for the detail explanation! I did feel something weird when adding Clone trait bound but couldn't tell why.

I also peeked at std::HashSet after the first implementation and found the use of Union and Difference type, which borrows from the input set. However, adding dedicated types for union() and difference() might be a little bit overkill for the book. So I'll probably first try to refactor it to return a lazy iterator.

On the other hand, I could still mention the first implementation in the chapter (updated in the dract) to demostrate how that can be refactored in a more 'rust' way (thanks to the explaination above!).

[weihanglo]

A step-by-step guide is highly appreciated. It would be a great experience to walk through implementation details with such a veteran Rust developer.

[weihanglo]

Great work! Would be merged.