CustomSet exercise allows creating new CustomSets with duplicated members

[mnquintana]

👋🏽 I came across an edge case that (I think) currently isn't being caught by the test cases for CustomSet – it's possible to write a CustomSet::new constructor that internally stores set members with duplicates, e.g.:

#[derive(Clone, Debug)]
pub struct CustomSet<T> {
    members: Vec<T>,
}

impl<T: Clone + PartialEq> CustomSet<T> {
    pub fn new(input: &[T]) -> Self {
        let members = input.to_vec();
        Self { members }
    }
}

On the one hand, I don't think the internal representation matters that much as long as the CustomSets methods account for duplicates.

But, after a cursory glance at the test cases, it looks like there aren't any test cases that instantiate a CustomSet from a slice with duplicate elements, which if I'm reading them right means a big chunk of a Set's functionality is currently untested (and the test suite passes solutions that don't actually dedupe members).

Example test case:

#[test]
fn intersection_of_two_sets_with_no_shared_elements_is_an_empty_set() {
    let set1 = CustomSet::new(&[1, 3, 2, 3, 3]);
    let set2 = CustomSet::new(&[4, 5, 6]);
    assert_eq!(set1.intersection(&set2), CustomSet::new(&[]));
    assert_eq!(set2.intersection(&set1), CustomSet::new(&[]));
}

Would y'all be open to a PR that adds test cases that test for CustomSet's deduping ability by passing in slices with duplicate elements?

[coriolinus]

I think that a deduplication test is a great idea! I'd be very open to a pr adding one.

…

On Sat, Jun 5, 2021, 18:06 Machisté N. Quintana ***@***.***> wrote: 👋🏽 I came across an edge case that (I think) currently isn't being caught by the test cases for CustomSet <https://exercism.io/tracks/rust/exercises/custom-set> – it's possible to write a CustomSet::new constructor that internally stores set members with duplicates, e.g.: #[derive(Clone, Debug)] pub struct CustomSet<T> { members: Vec<T>, } impl<T: Clone + PartialEq> CustomSet<T> { pub fn new(input: &[T]) -> Self { let members = input.to_vec(); Self { members } } } On the one hand, I don't think the internal representation matters that much as long as the CustomSets methods account for duplicates. But, after a cursory glance at the test cases <https://github.com/exercism/rust/blob/main/exercises/practice/custom-set/tests/custom-set.rs>, it looks like there aren't any test cases that instantiate a CustomSet from a slice with duplicate elements, which if I'm reading them right means a big chunk of a Set's functionality is currently untested (and the test suite passes solutions that don't actually dedupe members). Would y'all be open to a PR that adds test cases that test for CustomSet's deduping ability by passing in slices with duplicate elements? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1275>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3V4TQTAJADFWZK3YBXGZLTRJDSDANCNFSM46EW7FYA> .

[senekor]

should be fixed by exercism/problem-specifications#2324