Skip to content
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

feat: remove unneeded trait bound #545

Conversation

conradludgate
Copy link

tuple_combinations has an unnecessary trait bound.

This blocks certain iterators being able to use the extension. For instance, Drain does not implement Clone, since it takes a mutable reference of the Vec.

tuple_combinations has an unnecessary trait bound.
@conradludgate conradludgate marked this pull request as draft May 17, 2021 15:36
@conradludgate
Copy link
Author

Seems implementations do need Clone currently. I think this is unnecessary so I will continue working on this to remove the need.

@conradludgate
Copy link
Author

I've come up with a combination algorithm that modifies a slice in place to generate all combinations. This would mean that the iterator can collect all the elements from the iterator once, then clone those elements when they're ready to be returned

@conradludgate
Copy link
Author

sample code for my algorithm. I'll turn it into an iterator now :)

fn main() {
    let mut v = vec![1, 2, 3, 4, 5, 6];
    let n = 3;

    let c = choose(v.len(), n);
    println!("c = {}", c);
    for i in 1..=c {
        out(&v, n);
        combi(&mut v, n, i);
    }
}

pub fn out(elems: &[usize], n: usize) {
    println!("{:?}, ({:?})", &elems[..n], &elems[n..]);
}

fn choose(n: usize, r: usize) -> usize {
    (r+1..=n).product::<usize>() / (1..=(n-r)).product::<usize>()
}

pub fn combi<T: std::fmt::Debug>(elems: &mut [T], r: usize, step: usize) {
    let n = elems.len();
    
    if r == 1 {
        elems.rotate_left(1);
        return;
    }

    let steps_a = choose(n-1, r-1);
    let steps = choose(n, r);

    if step < steps_a {
        combi(&mut elems[1..], r-1, step);
    } else if step == steps_a {
        combi(&mut elems[1..], r-1, step);
        elems.rotate_left(1);
    } else if step < steps {
        combi(&mut elems[..n - 1], r, step - steps_a);
    } else {
        elems[..r].reverse();
        elems.reverse();
    }
}

@conradludgate
Copy link
Author

Iterator PoC here: https://gist.github.com/conradludgate/0b3fd7c3d9b5bc304fa84e8444327e50 It's very janky atm but it works and it provable. I want to try and get rid of the use of pointers though and package it better

@SkiFire13
Copy link
Contributor

I think you can implement this like the normal combinations, except with a tuple/array instead of the Vec for indices

pub struct Combinations<I: Iterator> {
indices: Vec<usize>,
pool: LazyBuffer<I>,
first: bool,
}

@conradludgate conradludgate deleted the tuple-combinations-no-clone branch May 23, 2021 12:21
@conradludgate
Copy link
Author

@SkiFire13 good point... completely slipped my mind. I've re-implemented and opened #546 instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants