Skip to content

Add internal_queue #13

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

Merged
41 changes: 41 additions & 0 deletions src/internal_queue.rs
Original file line number Diff line number Diff line change
@@ -1 +1,42 @@
struct SimpleQueue<T> {
payload: Vec<T>,
pos: usize,
}

impl<T> SimpleQueue<T>
where
T: Copy,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we want T to be Copy? It seems to me that passing T itself instead of reference &T in push is more natural.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it was what the original code did. But yes, it's so unnatural and complicated to write in Rust, so I'll fix it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think removing Copy is better in the normal Rust libraries, but considering this is a internal code and is only used in the implementation of the maxflow algorithm (only for integer and pair of integers), I also think we can restrict the level of abstraction. Original C++ implementation are not using move semantics, either. For more general purposes we should use VecDeque in my opinion. At least it doesn't have enough interface to use with non-Copy type, e.g. no way to get the ownership of the queued value back.
As an alternative, we can use T directly instead of using references &T if T: Copy to make it more natural.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for late response....

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. At least I changed &T to T in push method. Also it seemed that removing T: Copy does not do any harm, so I simply removed it for now.

{
fn reserve(&mut self, n: i32) {
let n = n as usize;
if n > self.payload.len() {
self.payload.reserve(n - self.payload.len());
}
}

fn size(&self) -> i32 {
(self.payload.len() - self.pos) as i32
}

fn empty(&self) -> bool {
self.pos == self.payload.len()
}

fn push(&mut self, t: &T) {
self.payload.push(*t);
}

// Do we need mutable version?
fn front(&self) -> &T {
&self.payload[self.pos]
}

fn clear(&mut self) {
self.payload.clear();
self.pos = 0;
}

fn pop(&mut self) {
self.pos += 1;
}
}