CustomSet exercise allows creating new `CustomSet`s with duplicated members
👋🏽 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?
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 https://github.com/exercism/rust/issues/1275, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3V4TQTAJADFWZK3YBXGZLTRJDSDANCNFSM46EW7FYA .
should be fixed by https://github.com/exercism/problem-specifications/pull/2324